diff --git a/include/swift/SIL/SILValue.h b/include/swift/SIL/SILValue.h index d814335e3f2f0..0e786cd2ff202 100644 --- a/include/swift/SIL/SILValue.h +++ b/include/swift/SIL/SILValue.h @@ -92,6 +92,10 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os, /// have. struct ValueOwnershipKind { enum innerty : uint8_t { + /// A value used to signal that two merged ValueOwnershipKinds were + /// incompatible. + Invalid = 0, + /// A SILValue with `Unowned` ownership kind is an independent value that /// has a lifetime that is only guaranteed to last until the next program /// visible side-effect. To maintain the lifetime of an unowned value, it @@ -156,7 +160,10 @@ struct ValueOwnershipKind { return Value == b; } - Optional merge(ValueOwnershipKind RHS) const; + /// Returns true if this ValueOwnershipKind is not invalid. + explicit operator bool() const { return Value != Invalid; } + + ValueOwnershipKind merge(ValueOwnershipKind RHS) const; /// Given that there is an aggregate value (like a struct or enum) with this /// ownership kind, and a subobject of type Proj is being projected from the @@ -172,6 +179,8 @@ struct ValueOwnershipKind { /// kinds. UseLifetimeConstraint getForwardingLifetimeConstraint() const { switch (Value) { + case ValueOwnershipKind::Invalid: + llvm_unreachable("Invalid ownership doesnt have a lifetime constraint!"); case ValueOwnershipKind::None: case ValueOwnershipKind::Guaranteed: case ValueOwnershipKind::Unowned: @@ -188,7 +197,7 @@ struct ValueOwnershipKind { /// The reason why we do not compare directy is to allow for /// ValueOwnershipKind::None to merge into other forms of ValueOwnershipKind. bool isCompatibleWith(ValueOwnershipKind other) const { - return merge(other).hasValue(); + return bool(merge(other)); } /// Returns isCompatibleWith(other.getOwnershipKind()). @@ -197,16 +206,14 @@ struct ValueOwnershipKind { /// dependencies. bool isCompatibleWith(SILValue other) const; - template - static Optional merge(RangeTy &&r) { - auto initial = Optional(ValueOwnershipKind::None); - return accumulate( - std::forward(r), initial, - [](Optional acc, ValueOwnershipKind x) { - if (!acc) - return acc; - return acc.getValue().merge(x); - }); + template static ValueOwnershipKind merge(RangeTy &&r) { + auto initial = ValueOwnershipKind::None; + return accumulate(std::forward(r), initial, + [](ValueOwnershipKind acc, ValueOwnershipKind x) { + if (!acc) + return acc; + return acc.merge(x); + }); } StringRef asString() const; diff --git a/lib/SIL/IR/SILInstructions.cpp b/lib/SIL/IR/SILInstructions.cpp index 05bbe5e2f0578..605899a6abac2 100644 --- a/lib/SIL/IR/SILInstructions.cpp +++ b/lib/SIL/IR/SILInstructions.cpp @@ -2886,5 +2886,8 @@ ReturnInst::ReturnInst(SILFunction &func, SILDebugLocation debugLoc, }); // Then merge all of our ownership kinds. Assert if we fail to merge. - ownershipKind = *ValueOwnershipKind::merge(ownershipKindRange); + ownershipKind = ValueOwnershipKind::merge(ownershipKindRange); + assert(ownershipKind != ValueOwnershipKind::Invalid && + "Conflicting ownership kinds when creating term inst from function " + "result info?!"); } diff --git a/lib/SIL/IR/SILValue.cpp b/lib/SIL/IR/SILValue.cpp index f4adab4cc7ab9..348782d940526 100644 --- a/lib/SIL/IR/SILValue.cpp +++ b/lib/SIL/IR/SILValue.cpp @@ -198,6 +198,8 @@ ValueOwnershipKind::ValueOwnershipKind(const SILFunction &F, SILType Type, StringRef ValueOwnershipKind::asString() const { switch (Value) { + case ValueOwnershipKind::Invalid: + return "invalid"; case ValueOwnershipKind::Unowned: return "unowned"; case ValueOwnershipKind::Owned: @@ -215,21 +217,27 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os, return os << kind.asString(); } -Optional -ValueOwnershipKind::merge(ValueOwnershipKind RHS) const { - auto LHSVal = Value; - auto RHSVal = RHS.Value; +ValueOwnershipKind ValueOwnershipKind::merge(ValueOwnershipKind rhs) const { + auto lhsVal = Value; + auto rhsVal = rhs.Value; - // Any merges with anything. - if (LHSVal == ValueOwnershipKind::None) { - return ValueOwnershipKind(RHSVal); - } - // Any merges with anything. - if (RHSVal == ValueOwnershipKind::None) { - return ValueOwnershipKind(LHSVal); - } + // If either lhs or rhs are invalid, return invalid. + if (lhsVal == ValueOwnershipKind::Invalid || + rhsVal == ValueOwnershipKind::Invalid) + return ValueOwnershipKind::Invalid; + + // None merges with anything. + if (lhsVal == ValueOwnershipKind::None) + return ValueOwnershipKind(rhsVal); + if (rhsVal == ValueOwnershipKind::None) + return ValueOwnershipKind(lhsVal); - return (LHSVal == RHSVal) ? Optional(*this) : llvm::None; + // At this point, if the two ownership kinds don't line up, the merge fails. + if (lhsVal != rhsVal) + return ValueOwnershipKind::Invalid; + + // Otherwise, we are good, return *this. + return *this; } ValueOwnershipKind::ValueOwnershipKind(StringRef S) { diff --git a/lib/SIL/IR/ValueOwnership.cpp b/lib/SIL/IR/ValueOwnership.cpp index 7ecc8ad741793..3d9ac9354c7c6 100644 --- a/lib/SIL/IR/ValueOwnership.cpp +++ b/lib/SIL/IR/ValueOwnership.cpp @@ -225,7 +225,7 @@ ValueOwnershipKindClassifier::visitForwardingInst(SILInstruction *i, return op.get().getOwnershipKind(); })); - if (!mergedValue.hasValue()) { + if (!mergedValue) { // If we have mismatched SILOwnership and sil ownership is not enabled, // just return None for staging purposes. If SILOwnership is enabled, then // we must assert! @@ -235,7 +235,7 @@ ValueOwnershipKindClassifier::visitForwardingInst(SILInstruction *i, llvm_unreachable("Forwarding inst with mismatching ownership kinds?!"); } - return mergedValue.getValue(); + return mergedValue; } #define FORWARDING_OWNERSHIP_INST(INST) \ @@ -341,7 +341,7 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) { llvm_unreachable("Forwarding inst with mismatching ownership kinds?!"); } - return *mergedOwnershipKind; + return mergedOwnershipKind; } ValueOwnershipKind ValueOwnershipKindClassifier::visitLoadInst(LoadInst *LI) { diff --git a/lib/SIL/Verifier/SILOwnershipVerifier.cpp b/lib/SIL/Verifier/SILOwnershipVerifier.cpp index 691b60936c3b5..7f10e9f90d163 100644 --- a/lib/SIL/Verifier/SILOwnershipVerifier.cpp +++ b/lib/SIL/Verifier/SILOwnershipVerifier.cpp @@ -487,6 +487,8 @@ bool SILValueOwnershipChecker::gatherUsers( bool SILValueOwnershipChecker::checkFunctionArgWithoutLifetimeEndingUses( SILFunctionArgument *arg) { switch (arg->getOwnershipKind()) { + case ValueOwnershipKind::Invalid: + llvm_unreachable("Invalid ownership kind used?!"); case ValueOwnershipKind::Guaranteed: case ValueOwnershipKind::Unowned: case ValueOwnershipKind::None: @@ -507,6 +509,8 @@ bool SILValueOwnershipChecker::checkFunctionArgWithoutLifetimeEndingUses( bool SILValueOwnershipChecker::checkYieldWithoutLifetimeEndingUses( BeginApplyResult *yield, ArrayRef regularUses) { switch (yield->getOwnershipKind()) { + case ValueOwnershipKind::Invalid: + llvm_unreachable("Invalid ownership kind used?!"); case ValueOwnershipKind::Unowned: case ValueOwnershipKind::None: return true; diff --git a/lib/SILGen/RValue.cpp b/lib/SILGen/RValue.cpp index 9eaf85abb2df3..f54a18c28c35e 100644 --- a/lib/SILGen/RValue.cpp +++ b/lib/SILGen/RValue.cpp @@ -388,7 +388,7 @@ static void verifyHelper(ArrayRef values, NullablePtr SGF = nullptr) { // This is a no-op in non-assert builds. #ifndef NDEBUG - auto result = Optional(ValueOwnershipKind::None); + ValueOwnershipKind result = ValueOwnershipKind::None; Optional sameHaveCleanups; for (ManagedValue v : values) { assert((!SGF || !v.getType().isLoadable(SGF.get()->F) || @@ -408,8 +408,8 @@ static void verifyHelper(ArrayRef values, // This variable is here so that if the assert below fires, the current // reduction value is still available. - auto newResult = result.getValue().merge(kind); - assert(newResult.hasValue()); + auto newResult = result.merge(kind); + assert(newResult); result = newResult; } #endif diff --git a/lib/SILOptimizer/Differentiation/Thunk.cpp b/lib/SILOptimizer/Differentiation/Thunk.cpp index cf9928673fdbb..65cb11bb409cd 100644 --- a/lib/SILOptimizer/Differentiation/Thunk.cpp +++ b/lib/SILOptimizer/Differentiation/Thunk.cpp @@ -496,6 +496,8 @@ SILFunction *getOrCreateReabstractionThunk(SILOptFunctionBuilder &fb, // Owned values need to be destroyed. for (auto arg : valuesToCleanup) { switch (arg.getOwnershipKind()) { + case ValueOwnershipKind::Invalid: + llvm_unreachable("Invalid ownership kind?!"); case ValueOwnershipKind::Guaranteed: builder.emitEndBorrowOperation(loc, arg); break; diff --git a/test/SIL/ownership-verifier/undef.sil b/test/SIL/ownership-verifier/undef.sil index e31d12e0a5f87..ab3d5262da7aa 100644 --- a/test/SIL/ownership-verifier/undef.sil +++ b/test/SIL/ownership-verifier/undef.sil @@ -20,6 +20,7 @@ struct MyInt { // CHECK-NEXT: Operand Ownership Map: // CHECK-NEXT: Op #: 0 // CHECK-NEXT: Ownership Map: -- OperandOwnershipKindMap -- +// CHECK-NEXT: invalid: No. // CHECK-NEXT: unowned: No. // CHECK-NEXT: owned: Yes. Liveness: LifetimeEnding // CHECK-NEXT: guaranteed: No. @@ -28,6 +29,7 @@ struct MyInt { // CHECK-NEXT: Operand Ownership Map: // CHECK-NEXT: Op #: 0 // CHECK-NEXT: Ownership Map: -- OperandOwnershipKindMap -- +// CHECK-NEXT: invalid: No. // CHECK-NEXT: unowned: No. // CHECK-NEXT: owned: Yes. Liveness: LifetimeEnding // CHECK-NEXT: guaranteed: No. @@ -36,6 +38,7 @@ struct MyInt { // CHECK-NEXT: Operand Ownership Map: // CHECK-NEXT: Op #: 0 // CHECK-NEXT: Ownership Map: -- OperandOwnershipKindMap -- +// CHECK-NEXT: invalid: No. // CHECK-NEXT: unowned: No. // CHECK-NEXT: owned: Yes. Liveness: LifetimeEnding // CHECK-NEXT: guaranteed: No. @@ -44,6 +47,7 @@ struct MyInt { // CHECK-NEXT: Operand Ownership Map: // CHECK-NEXT: Op #: 0 // CHECK-NEXT: Ownership Map: -- OperandOwnershipKindMap -- +// CHECK-NEXT: invalid: No. // CHECK-NEXT: unowned: No. // CHECK-NEXT: owned: No. // CHECK-NEXT: guaranteed: No.