Merge pull request swiftlang#78457 from jckarter/addressable-for-depedencies-2

jckarter · web-flow · commit bc405c5f0aa3 · 2025-01-08T07:49:56.000-08:00
`@_addressableForDependencies` fixes
diff --git a/include/swift/SIL/TypeLowering.h b/include/swift/SIL/TypeLowering.h
@@ -222,7 +222,7 @@ class TypeLowering {
             IsNotTypeExpansionSensitive,
         HasRawPointer_t hasRawPointer = DoesNotHaveRawPointer,
         IsLexical_t isLexical = IsNotLexical, HasPack_t hasPack = HasNoPack,
-        IsAddressableForDependencies_t isAFD = IsAddressableForDependencies)
+        IsAddressableForDependencies_t isAFD = IsNotAddressableForDependencies)
         : Flags((isTrivial ? 0U : NonTrivialFlag) |
                 (isFixedABI ? 0U : NonFixedABIFlag) |
                 (isAddressOnly ? AddressOnlyFlag : 0U) |
diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp
@@ -1779,20 +1779,17 @@ class DestructureInputs {
              ParameterTypeFlags origFlags) {
     assert(!isa<InOutType>(substType));
 
-    if (TC.Context.LangOpts.hasFeature(Feature::AddressableTypes)
-        || TC.Context.LangOpts.hasFeature(Feature::AddressableParameters)) {
-      // If the parameter is marked addressable, lower it with maximal
-      // abstraction.
-      if (origFlags.isAddressable()) {
+    // If the parameter is marked addressable, lower it with maximal
+    // abstraction.
+    if (origFlags.isAddressable()) {
+      origType = AbstractionPattern::getOpaque();
+    } else if (hasScopedDependency) {
+      // If there is a scoped dependency on this parameter, and the parameter
+      // is addressable-for-dependencies, then lower it with maximal abstraction
+      // as well.
+      auto &initialSubstTL = TC.getTypeLowering(origType, substType, expansion);
+      if (initialSubstTL.getRecursiveProperties().isAddressableForDependencies()) {
         origType = AbstractionPattern::getOpaque();
-      } else if (hasScopedDependency) {
-        // If there is a scoped dependency on this parameter, and the parameter
-        // is addressable-for-dependencies, then lower it with maximal abstraction
-        // as well.
-        auto &initialSubstTL = TC.getTypeLowering(origType, substType, expansion);
-        if (initialSubstTL.getRecursiveProperties().isAddressableForDependencies()) {
-          origType = AbstractionPattern::getOpaque();
-        }
       }
     }
 
diff --git a/lib/SIL/IR/TypeLowering.cpp b/lib/SIL/IR/TypeLowering.cpp
@@ -2510,6 +2510,7 @@ namespace {
       }
 
       if (D->isCxxNonTrivial()) {
+        properties.setAddressableForDependencies();
         properties.setAddressOnly();
         properties.setNonTrivial();
         properties.setLexical(IsLexical);
@@ -2550,6 +2551,10 @@ namespace {
         properties.setLexical(IsLexical);
         return handleAddressOnly(structType, properties);
       }
+      
+      if (D->getAttrs().hasAttribute<AddressableForDependenciesAttr>()) {
+        properties.setAddressableForDependencies();
+      }
 
       auto subMap = structType->getContextSubstitutionMap();
 
@@ -2620,6 +2625,10 @@ namespace {
       if (handleResilience(enumType, D, properties))
         return handleAddressOnly(enumType, properties);
 
+      if (D->getAttrs().hasAttribute<AddressableForDependenciesAttr>()) {
+        properties.setAddressableForDependencies();
+      }
+
       // [is_or_contains_pack_unsubstituted] Visit the elements of the
       // unsubstituted type to find pack types which would be substituted away.
       for (auto elt : D->getAllElements()) {
diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp
@@ -5015,23 +5015,22 @@ SILValue SILGenFunction::emitSemanticLoad(SILLocation loc,
   }
 
   // Harder case: the srcTL and the rvalueTL match without move only.
-  if (srcType.removingMoveOnlyWrapper() ==
-      rvalueType.removingMoveOnlyWrapper()) {
+  if (srcType.removingMoveOnlyWrapper()
+                                      == rvalueType.removingMoveOnlyWrapper()) {
     // Ok, we know that one must be move only and the other must not be. Thus we
     // perform one of two things:
     //
     // 1. If our source address is move only and our rvalue type is not move
-    // only, lets perform a load [copy] and a moveonly_to_copyable. We just need
-    // to insert something so that the move only checker knows that this copy of
-    // the move only address must be a last use.
+    // only, let's perform a load [copy] from the unwrapped address.
     //
     // 2. If our dest value type is move only and our rvalue type is not move
     // only, then we perform a load [copy] + copyable_to_moveonly.
-    SILValue newCopy = srcTL.emitLoadOfCopy(B, loc, src, isTake);
-    if (newCopy->getType().isMoveOnlyWrapped()) {
-      return B.createOwnedMoveOnlyWrapperToCopyableValue(loc, newCopy);
+    if (src->getType().isMoveOnlyWrapped()) {
+      auto copyableSrc = B.createMoveOnlyWrapperToCopyableAddr(loc, src);
+      return rvalueTL.emitLoadOfCopy(B, loc, copyableSrc, isTake);
     }
 
+    SILValue newCopy = srcTL.emitLoadOfCopy(B, loc, src, isTake);
     return B.createOwnedCopyableToMoveOnlyWrapperValue(loc, newCopy);
   }
 
diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp
@@ -210,9 +210,7 @@ class EmitBBArguments : public CanTypeVisitor<EmitBBArguments,
       return handleInOut(origType, substType, pd->isAddressable());
     }
     // Addressability also suppresses exploding the parameter.
-    if ((SGF.getASTContext().LangOpts.hasFeature(Feature::AddressableTypes)
-         || SGF.getASTContext().LangOpts.hasFeature(Feature::AddressableParameters))
-        && isAddressable) {
+    if (isAddressable) {
       return handleScalar(claimNextParameter(),
                           AbstractionPattern::getOpaque(), substType,
                           /*emitInto*/ nullptr,
@@ -740,15 +738,12 @@ class ArgumentInitHelper {
       // addressability can be implied by a scoped dependency.
       bool isAddressable = false;
       
-      if (SGF.getASTContext().LangOpts.hasFeature(Feature::AddressableTypes)
-          || SGF.getASTContext().LangOpts.hasFeature(Feature::AddressableParameters)) {
-        isAddressable = pd->isAddressable()
-          || (ScopedDependencies.contains(pd)
-              && SGF.getTypeLowering(origType, substType)
-                    .getRecursiveProperties().isAddressableForDependencies());
-        if (isAddressable) {
-          AddressableParams.insert(pd);
-        }
+      isAddressable = pd->isAddressable()
+        || (ScopedDependencies.contains(pd)
+            && SGF.getTypeLowering(origType, substType)
+                  .getRecursiveProperties().isAddressableForDependencies());
+      if (isAddressable) {
+        AddressableParams.insert(pd);
       }
       paramValue = argEmitter.handleParam(origType, substType, pd,
                                           isAddressable);
diff --git a/test/Concurrency/transfernonsendable_ownership.swift b/test/Concurrency/transfernonsendable_ownership.swift
@@ -50,20 +50,20 @@ func testConsumingError(_ x: consuming Klass) async {
   print(x)
 }
 
-func testConsumingUseAfterConsumeError(_ x: consuming Klass) async { // expected-error {{'x' consumed more than once}}
+func testConsumingUseAfterConsumeError(_ x: consuming Klass) async { // expected-error {{'x' used after consume}}
   await consumeTransferToMain(x) // expected-warning {{sending 'x' risks causing data races}}
   // expected-note @-1 {{sending task-isolated 'x' to main actor-isolated global function 'consumeTransferToMain' risks causing data races between main actor-isolated and task-isolated uses}}
   // expected-note @-2 {{consumed here}}
   print(x)
-  // expected-note @-1 {{consumed again here}}
+  // expected-note @-1 {{used here}}
 }
 
-@CustomActor func testConsumingUseAfterConsumeErrorGlobalActor(_ x: consuming Klass) async { // expected-error {{'x' consumed more than once}}
+@CustomActor func testConsumingUseAfterConsumeErrorGlobalActor(_ x: consuming Klass) async { // expected-error {{'x' used after consume}}
   await consumeTransferToMain(x) // expected-warning {{sending 'x' risks causing data races}}
   // expected-note @-1 {{sending global actor 'CustomActor'-isolated 'x' to main actor-isolated global function 'consumeTransferToMain' risks causing data races between main actor-isolated and global actor 'CustomActor'-isolated uses}}
   // expected-note @-2 {{consumed here}}
   print(x)
-  // expected-note @-1 {{consumed again here}}
+  // expected-note @-1 {{used here}}
 }
 
 func testBorrowing(_ x: borrowing Klass) async {
diff --git a/test/SILOptimizer/addressable_dependencies.swift b/test/SILOptimizer/addressable_dependencies.swift
@@ -6,6 +6,27 @@
 
 import Builtin
 
+struct NodeRef: ~Escapable {
+    private var parent: UnsafePointer<Node>
+
+    // CHECK-LABEL: sil {{.*}}@${{.*}}7NodeRefV4node{{.*}}fC :
+    // CHECK-SAME:    (@in_guaranteed Node,
+    @lifetime(borrow node)
+    init(node: borrowing Node) {
+        // CHECK: bb0(%0 : @noImplicitCopy $*Node,
+        // CHECK:   [[RAW_PTR:%.*]] = address_to_pointer {{.*}}%0
+        // CHECK:   struct $UnsafePointer<Node> ([[RAW_PTR]])
+        self.parent = UnsafePointer(Builtin.addressOfBorrow(node))
+    }
+
+    // CHECK-LABEL: sil {{.*}}@${{.*}}7NodeRefV9allocated{{.*}}fC :
+    // CHECK-SAME:    (@guaranteed AllocatedNode,
+    @lifetime(borrow allocated)
+    init(allocated: borrowing AllocatedNode) {
+        self.parent = allocated.node
+    }
+}
+
 @_addressableForDependencies
 struct Node {
     var id: String
@@ -23,16 +44,17 @@ struct Node {
     }
 }
 
-struct NodeRef: ~Escapable {
-    private var parent: UnsafePointer<Node>
+// not addressable for dependencies
+struct AllocatedNode: ~Copyable {
+    fileprivate var node: UnsafePointer<Node>
 
-    // CHECK-LABEL: sil {{.*}}@${{.*}}7NodeRefV4node{{.*}}fC :
-    // CHECK-SAME:    (@in_guaranteed Node,
-    @lifetime(borrow node)
-    init(node: borrowing Node) {
-        // CHECK: bb0(%0 : @noImplicitCopy $*Node,
-        // CHECK:   [[RAW_PTR:%.*]] = address_to_pointer {{.*}}%0
-        // CHECK:   struct $UnsafePointer<Node> ([[RAW_PTR]])
-        self.parent = UnsafePointer(Builtin.addressOfBorrow(node))
+    var ref: NodeRef {
+      // CHECK-LABEL: sil {{.*}}@${{.*}}13AllocatedNodeV3ref{{.*}}Vvg :
+      // CHECK-SAME:    (@guaranteed AllocatedNode) ->
+      @lifetime(borrow self)
+      borrowing get {
+        return NodeRef(allocated: self)
+      }
     }
 }
+
diff --git a/test/SILOptimizer/addressable_move_only_checking.swift b/test/SILOptimizer/addressable_move_only_checking.swift
@@ -0,0 +1,30 @@
+// RUN: %target-swift-frontend -emit-sil -enable-experimental-feature BuiltinModule -enable-experimental-feature LifetimeDependence -enable-experimental-feature AddressableTypes -enable-experimental-feature AddressableParameters -verify %s
+
+// REQUIRES: swift_feature_BuiltinModule
+// REQUIRES: swift_feature_AddressableParameters
+// REQUIRES: swift_feature_AddressableTypes
+// REQUIRES: swift_feature_LifetimeDependence
+
+@_addressableForDependencies
+struct Node {
+    var id: AnyObject
+
+    func grungle() {}
+}
+
+struct NodeRef: ~Escapable {
+    private var parent: UnsafePointer<Node>
+
+    @lifetime(borrow node)
+    init(node: borrowing Node) { fatalError() }
+}
+
+// Ensure there aren't spurious errors about consumption when an addressable
+// parameter is passed as a normal loadable parameter to another function
+// or method.
+@lifetime(borrow node)
+func test(node: borrowing Node) -> NodeRef {
+    node.grungle()
+    return NodeRef(node: node)
+}
+
diff --git a/test/SILOptimizer/noimplicitcopy_consuming_parameters.swift b/test/SILOptimizer/noimplicitcopy_consuming_parameters.swift
