Merge pull request #65604 from gottesmm/address-only-stuff

[move-only] Address Only Patches

Author: gottesmm

include/swift/SIL/SILUndef.h

@@ -30,6 +30,12 @@ class SILUndef : public ValueBase {
   void operator delete(void *, size_t) = delete;
 
   static SILUndef *get(SILType ty, SILModule &m);
+
+  /// Return a SILUndef with the same type as the passed in value.
+  static SILUndef *get(SILValue value) {
+    return SILUndef::get(value->getType(), *value->getModule());
+  }
+
   static SILUndef *get(SILType ty, const SILFunction &f);
 
   template <class OwnerTy>

lib/SILGen/SILGenLValue.cpp

@@ -3183,7 +3183,8 @@ RValue SILGenFunction::emitRValueForNonMemberVarDecl(SILLocation loc,
     SILValue accessAddr = UnenforcedFormalAccess::enter(*this, loc, destAddr,
                                                         SILAccessKind::Read);
 
-    if (accessAddr->getType().isMoveOnly()) {
+    if (accessAddr->getType().isMoveOnly() &&
+        !isa<MarkMustCheckInst>(accessAddr)) {
       // When loading an rvalue, we should never need to modify the place
       // we're loading from.
       accessAddr = B.createMarkMustCheckInst(

lib/SILGen/SILGenProlog.cpp

@@ -748,7 +748,6 @@ class ArgumentInitHelper {
   /// if not null.
   void makeArgumentIntoBinding(SILLocation loc, ParamDecl *pd) {
     ManagedValue argrv = makeArgument(loc, pd);
-
     SILValue value = argrv.getValue();
     if (pd->isInOut()) {
       assert(argrv.getType().isAddress() && "expected inout to be address");
@@ -768,18 +767,52 @@ class ArgumentInitHelper {
     if (!argrv.getType().isAddress()) {
       // NOTE: We setup SGF.VarLocs[pd] in updateArgumentValueForBinding.
       updateArgumentValueForBinding(argrv, loc, pd, value, varinfo);
-    } else {
-      if (auto *allocStack = dyn_cast<AllocStackInst>(value)) {
-        allocStack->setArgNo(ArgNo);
-        if (SGF.getASTContext().SILOpts.supportsLexicalLifetimes(
-                SGF.getModule()) &&
-            SGF.F.getLifetime(pd, value->getType()).isLexical())
-          allocStack->setIsLexical();
-      } else {
-        SGF.B.createDebugValueAddr(loc, value, varinfo);
-      }
+      return;
+    }
+
+    if (auto *allocStack = dyn_cast<AllocStackInst>(value)) {
+      allocStack->setArgNo(ArgNo);
+      if (SGF.getASTContext().SILOpts.supportsLexicalLifetimes(
+              SGF.getModule()) &&
+          SGF.F.getLifetime(pd, value->getType()).isLexical())
+        allocStack->setIsLexical();
       SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value);
+      return;
+    }
+
+    if (value->getType().isMoveOnly()) {
+      switch (pd->getValueOwnership()) {
+      case ValueOwnership::Default:
+        if (pd->isSelfParameter()) {
+          assert(!isa<MarkMustCheckInst>(value) &&
+                 "Should not have inserted mark must check inst in EmitBBArgs");
+          if (!pd->isInOut()) {
+            value = SGF.B.createMarkMustCheckInst(
+                loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
+          }
+        } else {
+          assert(isa<MarkMustCheckInst>(value) &&
+                 "Should have inserted mark must check inst in EmitBBArgs");
+        }
+        break;
+      case ValueOwnership::InOut:
+        assert(isa<MarkMustCheckInst>(value) &&
+               "Expected mark must check inst with inout to be handled in "
+               "emitBBArgs earlier");
+        break;
+      case ValueOwnership::Owned:
+        value = SGF.B.createMarkMustCheckInst(
+            loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
+        break;
+      case ValueOwnership::Shared:
+        value = SGF.B.createMarkMustCheckInst(
+            loc, value, MarkMustCheckInst::CheckKind::NoConsumeOrAssign);
+        break;
+      }
     }
+
+    SGF.B.createDebugValueAddr(loc, value, varinfo);
+    SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value);
   }
 
   void emitParam(ParamDecl *PD) {

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1511,6 +1511,19 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
     assert(op->getOperandNumber() == CopyAddrInst::Src &&
            "Should have dest above in memInstMust{Rei,I}nitialize");
 
+    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+    if (!leafRange)
+      return false;
+
+    // If we have a non-move only type, just treat this as a liveness use.
+    if (!copyAddr->getSrc()->getType().isMoveOnly()) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "Found copy of copyable type. Treating as liveness use! "
+                 << *user);
+      useState.livenessUses.insert({user, *leafRange});
+      return true;
+    }
+
     if (markedValue->getCheckKind() ==
         MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
       LLVM_DEBUG(llvm::dbgs()
@@ -1520,17 +1533,11 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
       return true;
     }
 
-    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
-    if (!leafRange)
-      return false;
-
     // TODO: Add borrow checking here like below.
 
     // TODO: Add destructure deinit checking here once address only checking is
     // completely brought up.
 
-    // TODO: Add check here that we don't error on trivial/copyable types.
-
     if (copyAddr->isTakeOfSrc()) {
       LLVM_DEBUG(llvm::dbgs() << "Found take: " << *user);
       useState.takeInsts.insert({user, *leafRange});
@@ -1721,9 +1728,30 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
   // Now that we have handled or loadTakeOrCopy, we need to now track our
   // additional pure takes.
   if (::memInstMustConsume(op)) {
+    // If we don't have a consumeable and assignable check kind, then we can't
+    // consume. Emit an error.
+    //
+    // NOTE: Since SILGen eagerly loads loadable types from memory, this
+    // generally will only handle address only types.
+    if (markedValue->getCheckKind() !=
+        MarkMustCheckInst::CheckKind::ConsumableAndAssignable) {
+      auto *fArg = dyn_cast<SILFunctionArgument>(
+          stripAccessMarkers(markedValue->getOperand()));
+      if (fArg && fArg->isClosureCapture() && fArg->getType().isAddress()) {
+        moveChecker.diagnosticEmitter.emitPromotedBoxArgumentError(markedValue,
+                                                                   fArg);
+      } else {
+        moveChecker.diagnosticEmitter
+            .emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
+      }
+      emittedEarlyDiagnostic = true;
+      return true;
+    }
+
     auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
     if (!leafRange)
       return false;
+
     LLVM_DEBUG(llvm::dbgs() << "Pure consuming use: " << *user);
     useState.takeInsts.insert({user, *leafRange});
     return true;
@@ -2423,7 +2451,6 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
     LLVM_DEBUG(llvm::dbgs() << "Failed access path visit: " << *markedAddress);
     return false;
   }
-  addressUseState.initializeInOutTermUsers();
 
   // If we found a load [copy] or copy_addr that requires multiple copies or an
   // exclusivity error, then we emitted an early error. Bail now and allow the
@@ -2438,9 +2465,14 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
   if (diagCount != diagnosticEmitter.getDiagnosticCount())
     return true;
 
-  // Then check if we emitted an error. If we did not, return true.
-  if (diagCount != diagnosticEmitter.getDiagnosticCount())
-    return true;
+  // Now that we know that we have run our visitor and did not emit any errors
+  // and successfully visited everything, see if have any
+  // assignable_but_not_consumable of address only types that are consumed.
+  //
+  // DISCUSSION: For non address only types, this is not an issue since we
+  // eagerly load
+
+  addressUseState.initializeInOutTermUsers();
 
   //===---
   // Liveness Checking

test/Interpreter/moveonly.swift

@@ -45,7 +45,7 @@ Tests.test("global destroyed once") {
   do {
     global = FD()
   }
-  expectEqual(0, LifetimeTracked.instances)    
+  expectEqual(0, LifetimeTracked.instances)
 }
 
 @_moveOnly
@@ -104,3 +104,31 @@ Tests.test("empty struct") {
     let _ = consume e
   }
 }
+
+protocol P {
+   var name: String { get }
+}
+
+Tests.test("AddressOnly") {
+    class Klass : P {
+        var name: String { "myName" }
+    }
+
+    @_moveOnly
+    struct S<T : P> {
+        var t: T
+    }
+
+    let e = S(t: Klass())
+    expectEqual(e.t.name, "myName")
+
+    func testGeneric<T : P>(_ x: borrowing S<T>) {
+        expectEqual(x.t.name, "myName")
+    }
+    testGeneric(e)
+
+    if e.t.name.count == 5 {
+        let _ = consume e
+    }
+}
+

test/SILGen/moveonly.swift

@@ -44,6 +44,18 @@ public enum NonTrivialEnum {
     case third(NonTrivialStruct)
 }
 
+@_moveOnly
+public struct AddressOnlyGeneric<T> {
+    var t: T
+}
+
+public protocol P {}
+
+@_moveOnly
+public struct AddressOnlyProtocol {
+    var t: P
+}
+
 var varGlobal = NonTrivialStruct()
 let letGlobal = NonTrivialStruct()
 
@@ -54,6 +66,8 @@ public func borrowVal(_ k: borrowing NonTrivialCopyableStruct) {}
 public func borrowVal(_ k: borrowing NonTrivialCopyableStruct2) {}
 public func borrowVal(_ s: borrowing NonTrivialStruct) {}
 public func borrowVal(_ s: borrowing NonTrivialStruct2) {}
+public func borrowVal<T>(_ s: borrowing AddressOnlyGeneric<T>) {}
+public func borrowVal(_ s: borrowing AddressOnlyProtocol) {}
 
 public func consumeVal(_ e : __owned NonTrivialEnum) {}
 public func consumeVal(_ e : __owned FD) {}
@@ -62,6 +76,8 @@ public func consumeVal(_ k: __owned NonTrivialCopyableStruct) {}
 public func consumeVal(_ k: __owned NonTrivialCopyableStruct2) {}
 public func consumeVal(_ s: __owned NonTrivialStruct) {}
 public func consumeVal(_ s: __owned NonTrivialStruct2) {}
+public func consumeVal<T>(_ s: __owned AddressOnlyGeneric<T>) {}
+public func consumeVal(_ s: __owned AddressOnlyProtocol) {}
 
 var bool: Bool { false }
 
@@ -137,6 +153,58 @@ public func useNonTrivialOwnedEnum(_ s: __owned NonTrivialEnum) {
     let _ = s2
 }
 
+// CHECK-LABEL: sil [ossa] @$s8moveonly21useAddressOnlyGenericyyAA0cdE0VyxGhlF : $@convention(thin) <T> (@in_guaranteed AddressOnlyGeneric<T>) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   mark_must_check [no_consume_or_assign] [[ARG]]
+// CHECK: } // end sil function '$s8moveonly21useAddressOnlyGenericyyAA0cdE0VyxGhlF'
+public func useAddressOnlyGeneric<T>(_ s: __shared AddressOnlyGeneric<T>) {
+    borrowVal(s)
+    let s2 = s
+    let k = s.t
+    let _ = k
+    borrowVal(s)
+    let _ = s2
+}
+
+// CHECK-LABEL: sil [ossa] @$s8moveonly26useOwnedAddressOnlyGenericyyAA0deF0VyxGnlF : $@convention(thin) <T> (@in AddressOnlyGeneric<T>) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   mark_must_check [consumable_and_assignable] [[ARG]]
+// CHECK: } // end sil function '$s8moveonly26useOwnedAddressOnlyGenericyyAA0deF0VyxGnlF'
+public func useOwnedAddressOnlyGeneric<T>(_ s: __owned AddressOnlyGeneric<T>) {
+    borrowVal(s)
+    let s2 = s
+    let k = s.t
+    let _ = k
+    borrowVal(s)
+    let _ = s2
+}
+
+// CHECK-LABEL: sil [ossa] @$s8moveonly22useAddressOnlyProtocolyyAA0cdE0VhF : $@convention(thin) (@in_guaranteed AddressOnlyProtocol) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   mark_must_check [no_consume_or_assign] [[ARG]]
+// CHECK: } // end sil function '$s8moveonly22useAddressOnlyProtocolyyAA0cdE0VhF'
+public func useAddressOnlyProtocol(_ s: __shared AddressOnlyProtocol) {
+    borrowVal(s)
+    let s2 = s
+    let k = s.t
+    let _ = k
+    borrowVal(s)
+    let _ = s2
+}
+
+// CHECK-LABEL: sil [ossa] @$s8moveonly27useOwnedAddressOnlyProtocolyyAA0deF0VnF : $@convention(thin) (@in AddressOnlyProtocol) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK:   mark_must_check [consumable_and_assignable] [[ARG]]
+// CHECK: } // end sil function '$s8moveonly27useOwnedAddressOnlyProtocolyyAA0deF0VnF'
+public func useOwnedAddressOnlyProtocol(_ s: __owned AddressOnlyProtocol) {
+    borrowVal(s)
+    let s2 = s
+    let k = s.t
+    let _ = k
+    borrowVal(s)
+    let _ = s2
+}
+
 //===---
 // Self in Init
 //
@@ -169,6 +237,73 @@ extension NonTrivialEnum {
     }
 }
 
+extension AddressOnlyGeneric {
+    // CHECK-LABEL: sil hidden [ossa] @$s8moveonly18AddressOnlyGenericV13testNoUseSelfyyF : $@convention(method) <T> (@in_guaranteed AddressOnlyGeneric<T>) -> () {
+    // CHECK: bb0([[ARG_IN:%.*]] :
+    // CHECK:   [[ARG:%.*]] = mark_must_check [no_consume_or_assign] [[ARG_IN]] :
+    //
+    // CHECK:   [[ALLOC_X:%.*]] = alloc_box $<τ_0_0> { let AddressOnlyGeneric<τ_0_0> } <T>, let, name "x"
+    // CHECK:   [[X:%.*]] = begin_borrow [lexical] [[ALLOC_X]]
+    // CHECK:   [[PROJECT_X:%.*]] = project_box [[X]]
+    // CHECK:   copy_addr [[ARG]] to [init] [[PROJECT_X]]
+    // CHECK:   [[MARKED_X:%.*]] = mark_must_check [no_consume_or_assign] [[PROJECT_X]]
+    // CHECK:   [[BLACKHOLE_ADDR:%.*]] = alloc_stack $AddressOnlyGeneric<T>
+    // CHECK:   copy_addr [[MARKED_X]] to [init] [[BLACKHOLE_ADDR]]
+    // CHECK:   destroy_addr [[BLACKHOLE_ADDR]]
+    // CHECK:   dealloc_stack [[BLACKHOLE_ADDR]]
+    //
+    // CHECK:   [[ALLOC_Y:%.*]] = alloc_box $<τ_0_0> { let AddressOnlyGeneric<τ_0_0> } <T>, let, name "y"
+    // CHECK:   [[Y:%.*]] = begin_borrow [lexical] [[ALLOC_Y]]
+    // CHECK:   [[PROJECT_Y:%.*]] = project_box [[Y]]
+    // CHECK:   copy_addr [[ARG]] to [init] [[PROJECT_Y]]
+    // CHECK:   [[MARKED_Y:%.*]] = mark_must_check [no_consume_or_assign] [[PROJECT_Y]]
+    // CHECK:   [[BLACKHOLE_ADDR:%.*]] = alloc_stack $AddressOnlyGeneric<T>
+    // CHECK:   copy_addr [[MARKED_Y]] to [init] [[BLACKHOLE_ADDR]]
+    // CHECK:   destroy_addr [[BLACKHOLE_ADDR]]
+    // CHECK:   dealloc_stack [[BLACKHOLE_ADDR]]
+    //
+    // CHECK: } // end sil function '$s8moveonly18AddressOnlyGenericV13testNoUseSelfyyF'
+    func testNoUseSelf() {
+        let x = self
+        let _ = x
+        let y = self
+        let _ = y
+    }
+}
+
+extension AddressOnlyProtocol {
+    // CHECK-LABEL: sil hidden [ossa] @$s8moveonly19AddressOnlyProtocolV13testNoUseSelfyyF : $@convention(method) (@in_guaranteed AddressOnlyProtocol) -> () {
+    // CHECK: bb0([[ARG_IN:%.*]] :
+    // CHECK:   [[ARG:%.*]] = mark_must_check [no_consume_or_assign] [[ARG_IN]] :
+    //
+    // CHECK:   [[ALLOC_X:%.*]] = alloc_box ${ let AddressOnlyProtocol }, let, name "x"
+    // CHECK:   [[X:%.*]] = begin_borrow [lexical] [[ALLOC_X]]
+    // CHECK:   [[PROJECT_X:%.*]] = project_box [[X]]
+    // CHECK:   copy_addr [[ARG]] to [init] [[PROJECT_X]]
+    // CHECK:   [[MARKED_X:%.*]] = mark_must_check [no_consume_or_assign] [[PROJECT_X]]
+    // CHECK:   [[BLACKHOLE_ADDR:%.*]] = alloc_stack $AddressOnlyProtocol
+    // CHECK:   copy_addr [[MARKED_X]] to [init] [[BLACKHOLE_ADDR]]
+    // CHECK:   destroy_addr [[BLACKHOLE_ADDR]]
+    // CHECK:   dealloc_stack [[BLACKHOLE_ADDR]]
+    //
+    // CHECK:   [[ALLOC_Y:%.*]] = alloc_box ${ let AddressOnlyProtocol }, let, name "y"
+    // CHECK:   [[Y:%.*]] = begin_borrow [lexical] [[ALLOC_Y]]
+    // CHECK:   [[PROJECT_Y:%.*]] = project_box [[Y]]
+    // CHECK:   copy_addr [[ARG]] to [init] [[PROJECT_Y]]
+    // CHECK:   [[MARKED_Y:%.*]] = mark_must_check [no_consume_or_assign] [[PROJECT_Y]]
+    // CHECK:   [[BLACKHOLE_ADDR:%.*]] = alloc_stack $AddressOnlyProtocol
+    // CHECK:   copy_addr [[MARKED_Y]] to [init] [[BLACKHOLE_ADDR]]
+    // CHECK:   destroy_addr [[BLACKHOLE_ADDR]]
+    // CHECK:   dealloc_stack [[BLACKHOLE_ADDR]]
+    // CHECK: } // end sil function '$s8moveonly19AddressOnlyProtocolV13testNoUseSelfyyF'
+    func testNoUseSelf() {
+        let x = self
+        let _ = x
+        let y = self
+        let _ = y
+    }
+}
+
 ///////////////////////////////
 // Black Hole Initialization //
 ///////////////////////////////