Merge pull request swiftlang#34659 from gottesmm/pr-5755ffcaf1433708dd6843521b63d70405f75d65

[ownership] Convert ValueOwnershipKind to have an Invalid state instead of using optional.

Author: gottesmm

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<ValueOwnershipKind> 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 <typename RangeTy>
-  static Optional<ValueOwnershipKind> merge(RangeTy &&r) {
-    auto initial = Optional<ValueOwnershipKind>(ValueOwnershipKind::None);
-    return accumulate(
-        std::forward<RangeTy>(r), initial,
-        [](Optional<ValueOwnershipKind> acc, ValueOwnershipKind x) {
-          if (!acc)
-            return acc;
-          return acc.getValue().merge(x);
-        });
+  template <typename RangeTy> static ValueOwnershipKind merge(RangeTy &&r) {
+    auto initial = ValueOwnershipKind::None;
+    return accumulate(std::forward<RangeTy>(r), initial,
+                      [](ValueOwnershipKind acc, ValueOwnershipKind x) {
+                        if (!acc)
+                          return acc;
+                        return acc.merge(x);
+                      });
   }
 
   StringRef asString() const;

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?!");
 }

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>
-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<ValueOwnershipKind>(*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) {

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) {

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<Operand *> regularUses) {
   switch (yield->getOwnershipKind()) {
+  case ValueOwnershipKind::Invalid:
+    llvm_unreachable("Invalid ownership kind used?!");
   case ValueOwnershipKind::Unowned:
   case ValueOwnershipKind::None:
     return true;

lib/SILGen/RValue.cpp

@@ -388,7 +388,7 @@ static void verifyHelper(ArrayRef<ManagedValue> values,
                          NullablePtr<SILGenFunction> SGF = nullptr) {
 // This is a no-op in non-assert builds.
 #ifndef NDEBUG
-  auto result = Optional<ValueOwnershipKind>(ValueOwnershipKind::None);
+  ValueOwnershipKind result = ValueOwnershipKind::None;
   Optional<bool> sameHaveCleanups;
   for (ManagedValue v : values) {
     assert((!SGF || !v.getType().isLoadable(SGF.get()->F) ||
@@ -408,8 +408,8 @@ static void verifyHelper(ArrayRef<ManagedValue> 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

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;

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.