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

At Andy's request. If it creates too much noise in covered switches, we may go
back to the original.

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.