Mo tuples mo problems

lib/SIL/SILValue.cpp

@@ -215,6 +215,7 @@ CONSTANT_OWNERSHIP_INST(Owned, PartialApply)
 CONSTANT_OWNERSHIP_INST(Owned, StrongPin)
 CONSTANT_OWNERSHIP_INST(Owned, ThinToThickFunction)
 CONSTANT_OWNERSHIP_INST(Owned, InitExistentialOpaque)
+CONSTANT_OWNERSHIP_INST(Owned, UnconditionalCheckedCastOpaque)
 
 // One would think that these /should/ be unowned. In truth they are owned since
 // objc metatypes do not go through the retain/release fast path. In their
@@ -409,6 +410,12 @@ ValueOwnershipKindVisitor::visitForwardingInst(SILInstruction *I,
 
     auto MergedValue = Base.merge(OpKind.Value);
     if (!MergedValue.hasValue()) {
+      // If we have mismatched SILOwnership and sil ownership is not enabled,
+      // just return Any for staging purposes. If SILOwnership is enabled, then
+      // we must assert!
+      if (!I->getModule().getOptions().EnableSILOwnership) {
+        return ValueOwnershipKind::Any;
+      }
       llvm_unreachable("Forwarding inst with mismatching ownership kinds?!");
     }
   }
@@ -433,7 +440,6 @@ FORWARDING_OWNERSHIP_INST(Struct)
 FORWARDING_OWNERSHIP_INST(Tuple)
 FORWARDING_OWNERSHIP_INST(UncheckedRefCast)
 FORWARDING_OWNERSHIP_INST(UnconditionalCheckedCast)
-FORWARDING_OWNERSHIP_INST(UnconditionalCheckedCastOpaque)
 FORWARDING_OWNERSHIP_INST(Upcast)
 FORWARDING_OWNERSHIP_INST(MarkUninitialized)
 FORWARDING_OWNERSHIP_INST(UncheckedEnumData)

lib/SIL/SILVerifier.cpp

@@ -1260,14 +1260,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               "store [init] or store [assign] can only be applied to "
               "non-trivial types");
       break;
-    case StoreOwnershipQualifier::Trivial:
+    case StoreOwnershipQualifier::Trivial: {
       require(
           F.hasQualifiedOwnership(),
           "Inst with qualified ownership in a function that is not qualified");
-      require(SI->getSrc()->getType().isTrivial(SI->getModule()),
-              "A store with trivial ownership must store a trivial type");
+      SILValue Src = SI->getSrc();
+      require(Src->getType().isTrivial(SI->getModule()) ||
+              Src.getOwnershipKind() == ValueOwnershipKind::Trivial,
+              "A store with trivial ownership must store a type with trivial "
+              "ownership");
       break;
     }
+    }
   }
 
   void checkAssignInst(AssignInst *AI) {

lib/SILGen/ManagedValue.h

@@ -76,6 +76,10 @@ class ManagedValue {
   ManagedValue(SILValue value, CleanupHandle cleanup)
     : valueAndFlag(value, false), cleanup(cleanup) {
     assert(value && "No value specified?!");
+    assert((!getType().isObject() ||
+            value.getOwnershipKind() != ValueOwnershipKind::Trivial ||
+            !hasCleanup()) &&
+           "Objects with trivial ownership should never have a cleanup");
   }
 
   /// Create a managed value for a +0 rvalue.
@@ -344,6 +348,9 @@ class ConsumableManagedValue {
 
   SILType getType() const { return Value.getType(); }
   SILValue getValue() const { return Value.getValue(); }
+  ValueOwnershipKind getOwnershipKind() const {
+    return Value.getOwnershipKind();
+  }
 
   /// Return a managed value appropriate for the final use of this CMV.
   ManagedValue getFinalManagedValue() const { return Value; }

lib/SILGen/RValue.cpp

@@ -81,7 +81,33 @@ class ExplodeTupleValue
     values.push_back(v);
   }
 
-  void visitTupleType(CanTupleType tupleFormalType, ManagedValue tupleMV) {
+  void visitObjectTupleType(CanTupleType tupleFormalType, ManagedValue tuple) {
+    bool isPlusZero = tuple.isPlusZeroRValueOrTrivial();
+    // SEMANTIC ARC TODO: This needs to be a take.
+    tuple = tuple.borrow(gen, loc);
+
+    for (auto i : indices(tupleFormalType->getElements())) {
+      CanType eltFormalType = tupleFormalType.getElementType(i);
+      assert(eltFormalType->isMaterializable());
+
+      auto eltTy = tuple.getType().getTupleElementType(i);
+      assert(eltTy.isAddress() == tuple.getType().isAddress());
+      auto &eltTI = gen.getTypeLowering(eltTy);
+
+      // Project the element.
+      assert(eltTI.isLoadable() || !gen.silConv.useLoweredAddresses());
+      ManagedValue elt = gen.B.createTupleExtract(loc, tuple, i, eltTy);
+      // If we're returning a +1 value, emit a cleanup for the member
+      // to cover for the cleanup we disabled for the tuple aggregate.
+      if (!isPlusZero)
+        elt = gen.B.createCopyValue(loc, elt);
+
+      visit(eltFormalType, elt);
+    }
+  }
+
+  void visitAddressTupleType(CanTupleType tupleFormalType,
+                             ManagedValue tupleMV) {
     bool isPlusZero = tupleMV.isPlusZeroRValueOrTrivial();
     SILValue tuple = tupleMV.forward(gen);
 
@@ -94,19 +120,13 @@ class ExplodeTupleValue
       auto &eltTI = gen.getTypeLowering(eltTy);
 
       // Project the element.
-      SILValue elt;
-      if (tuple->getType().isObject()) {
-        assert(eltTI.isLoadable() || !gen.silConv.useLoweredAddresses());
-        elt = gen.B.createTupleExtract(loc, tuple, i, eltTy);
-      } else {
-        elt = gen.B.createTupleElementAddr(loc, tuple, i, eltTy);
-
-        // RValue has an invariant that loadable values have been
-        // loaded.  Except it's not really an invariant, because
-        // argument emission likes to lie sometimes.
-        if (eltTI.isLoadable()) {
-          elt = eltTI.emitLoad(gen.B, loc, elt, LoadOwnershipQualifier::Take);
-        }
+      SILValue elt = gen.B.createTupleElementAddr(loc, tuple, i, eltTy);
+
+      // RValue has an invariant that loadable values have been
+      // loaded.  Except it's not really an invariant, because
+      // argument emission likes to lie sometimes.
+      if (eltTI.isLoadable()) {
+        elt = eltTI.emitLoad(gen.B, loc, elt, LoadOwnershipQualifier::Take);
       }
 
       // If we're returning a +1 value, emit a cleanup for the member
@@ -117,6 +137,14 @@ class ExplodeTupleValue
       visit(eltFormalType, eltMV);
     }
   }
+
+  void visitTupleType(CanTupleType tupleFormalType, ManagedValue tuple) {
+    if (tuple.getType().isObject()) {
+      return visitObjectTupleType(tupleFormalType, tuple);
+    }
+
+    visitAddressTupleType(tupleFormalType, tuple);
+  }
 };
 
 enum class ImplodeKind { Unmanaged, Forward, Copy };

lib/SILGen/SILGenBuilder.cpp

@@ -534,3 +534,19 @@ ManagedValue SILGenBuilder::createUpcast(SILLocation Loc, ManagedValue Original,
 
   return gen.emitManagedRValueWithCleanup(convertedValue);
 }
+
+ManagedValue SILGenBuilder::createTupleElementAddr(SILLocation Loc,
+                                                   ManagedValue Base,
+                                                   unsigned Index,
+                                                   SILType Type) {
+  SILValue TupleEltAddr =
+      SILBuilder::createTupleElementAddr(Loc, Base.getValue(), Index, Type);
+  return ManagedValue::forUnmanaged(TupleEltAddr);
+}
+
+ManagedValue SILGenBuilder::createTupleElementAddr(SILLocation Loc,
+                                                   ManagedValue Value,
+                                                   unsigned Index) {
+  SILType Type = Value.getType().getTupleElementType(Index);
+  return createTupleElementAddr(Loc, Value, Index, Type);
+}

lib/SILGen/SILGenBuilder.h

@@ -163,11 +163,15 @@ class SILGenBuilder : public SILBuilder {
                         ArrayRef<ManagedValue> elementCountOperands);
 
   using SILBuilder::createTupleExtract;
-
   ManagedValue createTupleExtract(SILLocation loc, ManagedValue value,
                                   unsigned index, SILType type);
   ManagedValue createTupleExtract(SILLocation loc, ManagedValue value,
                                   unsigned index);
+  using SILBuilder::createTupleElementAddr;
+  ManagedValue createTupleElementAddr(SILLocation loc, ManagedValue addr,
+                                      unsigned index, SILType type);
+  ManagedValue createTupleElementAddr(SILLocation loc, ManagedValue addr,
+                                      unsigned index);
 
   using SILBuilder::createLoadBorrow;
   ManagedValue createLoadBorrow(SILLocation loc, ManagedValue base);
@@ -196,9 +200,9 @@ class SILGenBuilder : public SILBuilder {
   ManagedValue createLoadTake(SILLocation loc, ManagedValue addr);
   ManagedValue createLoadTake(SILLocation loc, ManagedValue addr,
                               const TypeLowering &lowering);
-  ManagedValue createLoadCopy(SILLocation Loc, ManagedValue Addr);
-  ManagedValue createLoadCopy(SILLocation Loc, ManagedValue Addr,
-                              const TypeLowering &Lowering);
+  ManagedValue createLoadCopy(SILLocation loc, ManagedValue addr);
+  ManagedValue createLoadCopy(SILLocation loc, ManagedValue addr,
+                              const TypeLowering &lowering);
 
   ManagedValue createFunctionArgument(SILType type, ValueDecl *decl);
 

lib/SILGen/SILGenExpr.cpp

@@ -57,6 +57,9 @@ ManagedValue SILGenFunction::emitManagedRetain(SILLocation loc,
   assert(lowering.getLoweredType() == v->getType());
   if (lowering.isTrivial())
     return ManagedValue::forUnmanaged(v);
+  if (v->getType().isObject() &&
+      v.getOwnershipKind() == ValueOwnershipKind::Trivial)
+    return ManagedValue::forUnmanaged(v);
   assert(!lowering.isAddressOnly() && "cannot retain an unloadable type");
 
   v = lowering.emitCopyValue(B, loc, v);
@@ -74,6 +77,8 @@ ManagedValue SILGenFunction::emitManagedLoadCopy(SILLocation loc, SILValue v,
   v = lowering.emitLoadOfCopy(B, loc, v, IsNotTake);
   if (lowering.isTrivial())
     return ManagedValue::forUnmanaged(v);
+  if (v.getOwnershipKind() == ValueOwnershipKind::Trivial)
+    return ManagedValue::forUnmanaged(v);
   assert(!lowering.isAddressOnly() && "cannot retain an unloadable type");
   return emitManagedRValueWithCleanup(v, lowering);
 }
@@ -107,7 +112,8 @@ ManagedValue SILGenFunction::emitManagedStoreBorrow(SILLocation loc, SILValue v,
 ManagedValue SILGenFunction::emitManagedStoreBorrow(
     SILLocation loc, SILValue v, SILValue addr, const TypeLowering &lowering) {
   assert(lowering.getLoweredType().getObjectType() == v->getType());
-  if (lowering.isTrivial()) {
+  if (lowering.isTrivial() ||
+      v.getOwnershipKind() == ValueOwnershipKind::Trivial) {
     lowering.emitStore(B, loc, v, addr, StoreOwnershipQualifier::Trivial);
     return ManagedValue::forUnmanaged(v);
   }
@@ -118,8 +124,6 @@ ManagedValue SILGenFunction::emitManagedStoreBorrow(
 
 ManagedValue SILGenFunction::emitManagedBeginBorrow(SILLocation loc,
                                                     SILValue v) {
-  if (v.getOwnershipKind() == ValueOwnershipKind::Guaranteed)
-    return ManagedValue::forUnmanaged(v);
   auto &lowering = F.getTypeLowering(v->getType());
   return emitManagedBeginBorrow(loc, v, lowering);
 }
@@ -131,8 +135,13 @@ SILGenFunction::emitManagedBeginBorrow(SILLocation loc, SILValue v,
          v->getType().getObjectType());
   if (lowering.isTrivial())
     return ManagedValue::forUnmanaged(v);
+
+  if (v.getOwnershipKind() == ValueOwnershipKind::Trivial)
+    return ManagedValue::forUnmanaged(v);
+
   if (v.getOwnershipKind() == ValueOwnershipKind::Guaranteed)
     return ManagedValue::forUnmanaged(v);
+
   auto *bbi = B.createBeginBorrow(loc, v);
   return emitManagedBorrowedRValueWithCleanup(v, bbi, lowering);
 }
@@ -263,9 +272,14 @@ ManagedValue SILGenFunction::emitManagedBorrowedRValueWithCleanup(
   if (lowering.isTrivial())
     return ManagedValue::forUnmanaged(borrowed);
 
+  if (original->getType().isObject() &&
+      original.getOwnershipKind() == ValueOwnershipKind::Trivial)
+    return ManagedValue::forUnmanaged(borrowed);
+
   if (borrowed->getType().isObject()) {
     Cleanups.pushCleanup<EndBorrowCleanup>(original, borrowed);
   }
+
   return ManagedValue(borrowed, CleanupHandle::invalid());
 }
 
@@ -279,7 +293,10 @@ ManagedValue SILGenFunction::emitManagedRValueWithCleanup(SILValue v,
   assert(lowering.getLoweredType() == v->getType());
   if (lowering.isTrivial())
     return ManagedValue::forUnmanaged(v);
-
+  if (v->getType().isObject() &&
+      v.getOwnershipKind() == ValueOwnershipKind::Trivial) {
+    return ManagedValue::forUnmanaged(v);
+  }
   return ManagedValue(v, enterDestroyCleanup(v));
 }
 
@@ -1670,8 +1687,14 @@ ManagedValue SILGenFunction::getManagedValue(SILLocation loc,
   if (valueTL.isTrivial())
     return ManagedValue::forUnmanaged(value.getValue());
 
-  // If it's an object, retain and enter a release cleanup.
+  // If it's an object...
   if (valueTy.isObject()) {
+    // See if we have more accurate information from the ownership kind. This
+    // detects trivial cases of enums.
+    if (value.getOwnershipKind() == ValueOwnershipKind::Trivial)
+      return ManagedValue::forUnmanaged(value.getValue());
+
+    // Otherwise, retain and enter a release cleanup.
     valueTL.emitCopyValue(B, loc, value.getValue());
     return emitManagedRValueWithCleanup(value.getValue(), valueTL);
   }

test/SIL/ownership-verifier/opaque_use_verifier.sil

@@ -0,0 +1,16 @@
+// RUN: %target-sil-opt -enable-sil-opaque-values -enable-sil-ownership -enable-sil-verify-all=0 -module-name Swift -o /dev/null 2>&1  %s
+// REQUIRES: asserts
+
+// This file is meant to contain tests that previously the verifier treated
+// incorrectly. This is important to ensure that the verifier does not
+// regress. It should only deal with use matching of opaque types.
+
+sil_stage canonical
+
+import Builtin
+
+sil @unconditional_checked_cast_opaque_test : $@convention(thin) <T> (Builtin.Int32) -> @out T {
+bb0(%0 : @trivial $Builtin.Int32):
+  %1 = unconditional_checked_cast_opaque %0 : $Builtin.Int32 to $T
+  return %1 : $T
+}

test/SILGen/builtins.swift

@@ -795,9 +795,13 @@ func refcast_any_punknown(_ o: AnyObject) -> PUnknown {
 // CHECK:   [[BORROWED_P:%.*]] = begin_borrow [[P]]
 // CHECK:   [[P_COPY:%.*]] = copy_value  [[BORROWED_P]]
 // CHECK:   [[T:%.*]] = builtin "unsafeGuaranteed"<A>([[P_COPY]] : $A)
-// CHECK:   [[R:%.*]] = tuple_extract [[T]] : $(A, Builtin.Int8), 0
-// CHECK:   [[K:%.*]] = tuple_extract [[T]] : $(A, Builtin.Int8), 1
-// CHECK:   destroy_value [[R]] : $A
+// CHECK:   [[BORROWED_T:%.*]] = begin_borrow [[T]]
+// CHECK:   [[R:%.*]] = tuple_extract [[BORROWED_T]] : $(A, Builtin.Int8), 0
+// CHECK:   [[COPY_R:%.*]] = copy_value [[R]]
+// CHECK:   [[K:%.*]] = tuple_extract [[BORROWED_T]] : $(A, Builtin.Int8), 1
+// CHECK:   destroy_value [[COPY_R]] : $A
+// CHECK:   end_borrow [[BORROWED_T]] from [[T]]
+// CHECK:   destroy_value [[T]]
 // CHECK:   end_borrow [[BORROWED_P]] from [[P]]
 // CHECK:   [[BORROWED_P:%.*]] = begin_borrow [[P]]
 // CHECK:   [[P_COPY:%.*]] = copy_value [[BORROWED_P]]
@@ -817,9 +821,13 @@ func unsafeGuaranteed_class(_ a: A) -> A {
 // CHECK:   [[BORROWED_P:%.*]] = begin_borrow [[P]]
 // CHECK:   [[P_COPY:%.*]] = copy_value  [[BORROWED_P]]
 // CHECK:   [[T:%.*]] = builtin "unsafeGuaranteed"<T>([[P_COPY]] : $T)
-// CHECK:   [[R:%.*]] = tuple_extract [[T]] : $(T, Builtin.Int8), 0
-// CHECK:   [[K:%.*]] = tuple_extract [[T]] : $(T, Builtin.Int8), 1
-// CHECK:   destroy_value [[R]] : $T
+// CHECK:   [[BORROWED_T:%.*]] = begin_borrow [[T]]
+// CHECK:   [[R:%.*]] = tuple_extract [[BORROWED_T]] : $(T, Builtin.Int8), 0
+// CHECK:   [[COPY_R:%.*]] = copy_value [[R]]
+// CHECK:   [[K:%.*]] = tuple_extract [[BORROWED_T]] : $(T, Builtin.Int8), 1
+// CHECK:   destroy_value [[COPY_R]] : $T
+// CHECK:   end_borrow [[BORROWED_T]] from [[T]]
+// CHECK:   destroy_value [[T]]
 // CHECK:   end_borrow [[BORROWED_P]] from [[P]]
 // CHECK:   [[BORROWED_P:%.*]] = begin_borrow [[P]]
 // CHECK:   [[P_RETURN:%.*]] = copy_value [[BORROWED_P]]
@@ -837,11 +845,15 @@ func unsafeGuaranteed_generic<T: AnyObject> (_ a: T) -> T {
 // CHECK:   [[BORROWED_P:%.*]] = begin_borrow [[P]]
 // CHECK:   [[P_COPY:%.*]] = copy_value [[BORROWED_P]]
 // CHECK:   [[T:%.*]] = builtin "unsafeGuaranteed"<T>([[P_COPY]] : $T)
-// CHECK:   [[R]] = tuple_extract [[T]] : $(T, Builtin.Int8), 0
-// CHECK:   [[K]] = tuple_extract [[T]] : $(T, Builtin.Int8), 1
+// CHECK:   [[BORROWED_T:%.*]] = begin_borrow [[T]]
+// CHECK:   [[R]] = tuple_extract [[BORROWED_T]] : $(T, Builtin.Int8), 0
+// CHECK:   [[COPY_R:%.*]] = copy_value [[R]]
+// CHECK:   [[K]] = tuple_extract [[BORROWED_T]] : $(T, Builtin.Int8), 1
+// CHECK:   end_borrow [[BORROWED_T]] from [[T]]
+// CHECK:   destroy_value [[T]]
 // CHECK:   end_borrow [[BORROWED_P]] from [[P]]
 // CHECK:   destroy_value [[P]]
-// CHECK:   [[S:%.*]] = tuple ([[R]] : $T, [[K]] : $Builtin.Int8)
+// CHECK:   [[S:%.*]] = tuple ([[COPY_R]] : $T, [[K]] : $Builtin.Int8)
 // CHECK:   return [[S]] : $(T, Builtin.Int8)
 // CHECK: }
 func unsafeGuaranteed_generic_return<T: AnyObject> (_ a: T) -> (T, Builtin.Int8) {