[nonescapable] remove '@_lifetime' requirement on implicit accessors

atrick · atrick · commit d3002ed0b215 · 2025-06-24T15:06:30.000-07:00
This avoids diagnostic errors on synthesized accessors, which are impossible for developers to understand. Fixes rdar://153793344 (Lifetime-dependent value returned by generated accessor '_read') (cherry picked from commit 855b3e4)
diff --git a/lib/AST/LifetimeDependence.cpp b/lib/AST/LifetimeDependence.cpp
@@ -464,6 +464,7 @@ class LifetimeDependenceChecker {
     return false;
   }
 
+  // Infer ambiguous cases for backward compatibility.
   bool useLazyInference() const {
     return isInterfaceFile()
       || ctx.LangOpts.EnableExperimentalLifetimeDependenceInference;
@@ -935,7 +936,7 @@ class LifetimeDependenceChecker {
     // Infer non-Escapable results.
     if (isDiagnosedNonEscapable(getResultOrYield())) {
       if (hasImplicitSelfParam()) {
-        // Methods and accessors that return or yield a non-Escapable value.
+        // Methods that return a non-Escapable value.
         inferNonEscapableResultOnSelf();
         return;
       }
@@ -973,32 +974,43 @@ class LifetimeDependenceChecker {
     return true;
   }
 
-  // Infer method dependence: result depends on self. This includes _modify.
+  // Infer method dependence of result on self for
+  // methods, getters, and _modify accessors.
   void inferNonEscapableResultOnSelf() {
     auto *afd = cast<AbstractFunctionDecl>(decl);
     Type selfTypeInContext = dc->getSelfTypeInContext();
     if (selfTypeInContext->hasError()) {
       return;
     }
-    bool nonEscapableSelf = isDiagnosedNonEscapable(selfTypeInContext);
 
-    // Avoid diagnosing inference on mutating methods when 'self' is
-    // non-Escapable. The inout 'self' also needs an inferred dependence on
-    // itself. This will be diagnosed when checking for missing dependencies.
-    if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
-      if (auto accessor = dyn_cast<AccessorDecl>(afd)) {
-        inferMutatingAccessor(accessor);
+    bool nonEscapableSelf = isDiagnosedNonEscapable(selfTypeInContext);
+    if (auto accessor = dyn_cast<AccessorDecl>(afd)) {
+      if (isImplicitOrSIL() || useLazyInference()) {
+        if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
+          // Implicit accessors that return or yield a non-Escapable value may
+          // infer dependency on both self and result.
+          inferMutatingAccessor(accessor);
+        }
+        // Infer the result dependency on self based on the kind of accessor
+        // that is wrapped by this synthesized accessors.
+        if (auto dependenceKind =
+            getAccessorDependence(accessor, selfTypeInContext)) {
+          pushDeps(createDeps(resultIndex).add(selfIndex, *dependenceKind));
+        }
+        return;
       }
+      // Explicit accessors are inferred the same way as regular methods.
+    }
+    // Do infer the result of a mutating method when 'self' is
+    // non-Escapable. The missing dependence on inout 'self' will be diagnosed
+    // later anyway, so an explicit annotation will still be needed.
+    if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
       return;
     }
-
     // Methods with parameters only apply to lazy inference.
     if (!useLazyInference() && afd->getParameters()->size() > 0) {
       return;
     }
-    // Allow inference for implicit getters, which simply return a stored,
-    // property, and for implicit _read/_modify, which cannot be defined
-    // explicitly alongside a regular getter.
     if (!useLazyInference() && !isImplicitOrSIL()) {
       // Require explicit @_lifetime(borrow self) for UnsafePointer-like self.
       if (!nonEscapableSelf && isBitwiseCopyable(selfTypeInContext, ctx)) {
@@ -1014,9 +1026,12 @@ class LifetimeDependenceChecker {
         return;
       }
     }
+    // Infer based on ownership if possible for either explicit accessors or
+    // methods as long as they pass preceding ambiguity checks.
     auto kind = inferLifetimeDependenceKind(
         selfTypeInContext, afd->getImplicitSelfDecl()->getValueOwnership());
     if (!kind) {
+      // Special diagnostic for an attempt to depend on a consuming parameter.
       diagnose(returnLoc,
                diag::lifetime_dependence_cannot_infer_scope_ownership,
                "self", diagnosticQualifier());
@@ -1025,6 +1040,8 @@ class LifetimeDependenceChecker {
     pushDeps(createDeps(resultIndex).add(selfIndex, *kind));
   }
 
+  // Infer the kind of dependence that makes sense for reading or writing a
+  // stored property (for getters or initializers).
   std::optional<LifetimeDependenceKind>
   inferLifetimeDependenceKind(Type sourceType, ValueOwnership ownership) {
     auto *afd = cast<AbstractFunctionDecl>(decl);
@@ -1039,8 +1056,9 @@ class LifetimeDependenceChecker {
     auto loweredOwnership = ownership != ValueOwnership::Default
                                 ? ownership
                                 : getLoweredOwnership(afd);
-    if (loweredOwnership != ValueOwnership::Shared &&
-        loweredOwnership != ValueOwnership::InOut) {
+    // It is impossible to depend on a consumed Escapable value (unless it is
+    // BitwiseCopyable as checked above).
+    if (loweredOwnership == ValueOwnership::Owned) {
       return std::nullopt;
     }
     return LifetimeDependenceKind::Scope;
@@ -1194,7 +1212,10 @@ class LifetimeDependenceChecker {
     // Handle implicit setters before diagnosing mutating methods. This
     // does not include global accessors, which have no implicit 'self'.
     if (auto accessor = dyn_cast<AccessorDecl>(afd)) {
-      inferMutatingAccessor(accessor);
+      // Explicit setters require explicit lifetime dependencies.
+      if (isImplicitOrSIL() || useLazyInference()) {
+        inferMutatingAccessor(accessor);
+      }
       return;
     }
     if (afd->getParameters()->size() > 0) {
@@ -1211,13 +1232,31 @@ class LifetimeDependenceChecker {
                                        LifetimeDependenceKind::Inherit));
   }
 
-  // Infer a mutating accessor's non-Escapable 'self' dependencies.
-  void inferMutatingAccessor(AccessorDecl *accessor) {
+  // Infer dependence for an accessor whose non-escapable result depends on
+  // self. This includes _read and _modify.
+  void inferAccessor(AccessorDecl *accessor, Type selfTypeInContext) {
+    // Explicit accessors require explicit lifetime dependencies.
     auto *afd = cast<AbstractFunctionDecl>(decl);
     if (!isImplicitOrSIL() && !useLazyInference()) {
-      // Explicit setters require explicit lifetime dependencies.
       return;
     }
+    bool nonEscapableSelf = isDiagnosedNonEscapable(selfTypeInContext);
+    if (nonEscapableSelf && afd->getImplicitSelfDecl()->isInOut()) {
+      // First, infer the dependency on the inout non-Escapable self. This may
+      // result in two inferred dependencies for accessors.
+      inferMutatingAccessor(accessor);
+    }
+    // Infer the result dependency on self based on the kind of accessor that
+    // is wrapped by this synthesized accessors.
+    if (auto dependenceKind =
+        getAccessorDependence(accessor, selfTypeInContext)) {
+      pushDeps(createDeps(resultIndex).add(selfIndex, *dependenceKind));
+    }
+  }
+
+  // Infer a mutating accessor's non-Escapable 'self' dependencies.
+  void inferMutatingAccessor(AccessorDecl *accessor) {
+    auto *afd = cast<AbstractFunctionDecl>(decl);
     switch (accessor->getAccessorKind()) {
     case AccessorKind::Read:
     case AccessorKind::Read2:
@@ -1234,8 +1273,6 @@ class LifetimeDependenceChecker {
       // _modify for them even though it won't be emitted.
       pushDeps(createDeps(selfIndex).add(selfIndex,
                                          LifetimeDependenceKind::Inherit));
-      pushDeps(createDeps(resultIndex).add(selfIndex,
-                                           LifetimeDependenceKind::Scope));
       break;
     case AccessorKind::Set: {
       const unsigned newValIdx = 0;
@@ -1255,7 +1292,7 @@ class LifetimeDependenceChecker {
       // dependency, so the setter cannot depend on 'newValue'.
       if (!paramTypeInContext->isEscapable()) {
         targetDeps = std::move(targetDeps)
-          .add(newValIdx, LifetimeDependenceKind::Inherit);
+                         .add(newValIdx, LifetimeDependenceKind::Inherit);
       }
       pushDeps(std::move(targetDeps));
       break;
@@ -1277,6 +1314,55 @@ class LifetimeDependenceChecker {
     }
   }
 
+  // Implicit accessors must be consistent with the accessor that they
+  // wrap. Otherwise, the sythesized implementation will report a diagnostic
+  // error.
+  std::optional<LifetimeDependenceKind>
+  getAccessorDependence(AccessorDecl *accessor, Type selfTypeInContext) {
+    auto *afd = cast<AbstractFunctionDecl>(decl);
+    std::optional<AccessorKind> wrappedAccessorKind = std::nullopt;
+    switch (accessor->getAccessorKind()) {
+    case AccessorKind::Read:
+    case AccessorKind::Read2:
+    case AccessorKind::Modify:
+    case AccessorKind::Modify2:
+      // read/modify are syntesized as calls to the getter.
+      wrappedAccessorKind = AccessorKind::Get;
+      break;
+    case AccessorKind::Get:
+      // getters are synthesized as access to a stored property.
+      break;
+    default:
+      // Unknown synthesized accessor.
+      // Setters are handled in inferMutatingAccessor() because they don't
+      // return a value.
+      return std::nullopt;
+    }
+    if (wrappedAccessorKind) {
+      auto *var = cast<VarDecl>(accessor->getStorage());
+      for (auto *wrappedAccessor : var->getAllAccessors()) {
+        if (wrappedAccessor->isImplicit())
+          continue;
+        if (wrappedAccessor->getAccessorKind() == wrappedAccessorKind) {
+          if (auto deps = wrappedAccessor->getLifetimeDependencies()) {
+            for (auto &dep : *deps) {
+              if (dep.getTargetIndex() != resultIndex)
+                continue;
+              if (dep.checkInherit(selfIndex))
+                return LifetimeDependenceKind::Inherit;
+              if (dep.checkScope(selfIndex))
+                return LifetimeDependenceKind::Scope;
+            }
+          }
+        }
+      }
+    }
+    // Either a Get or Modify without any wrapped accessor. Handle these like a
+    // read of the stored property.
+    return inferLifetimeDependenceKind(
+      selfTypeInContext, afd->getImplicitSelfDecl()->getValueOwnership());
+  }
+
   // Infer 'inout' parameter dependency when the only parameter is
   // non-Escapable.
   //
diff --git a/test/SILOptimizer/lifetime_dependence/accessors.swift b/test/SILOptimizer/lifetime_dependence/accessors.swift
@@ -0,0 +1,81 @@
+// RUN: %target-swift-emit-sil -enable-experimental-feature Lifetimes -primary-file %s | %FileCheck %s
+
+// REQUIRES: swift_feature_Lifetimes
+
+struct NE : ~Escapable {
+  let _p: UnsafeRawPointer
+
+  @_lifetime(borrow p)
+  init(_ p: UnsafeRawPointer) { self._p = p }
+}
+
+struct NCNE : ~Copyable, ~Escapable {
+  let _p: UnsafeRawPointer
+
+  @_lifetime(borrow p)
+  init(_ p: UnsafeRawPointer) {
+    self._p = p
+  }
+}
+
+struct NEStoredProp : ~Escapable {
+  var ne: NE
+  // NEStoredProp.ne.getter
+  // CHECK-LABEL: sil hidden [transparent] @$s9accessors12NEStoredPropV2neAA2NEVvg : $@convention(method) (@guaranteed NEStoredProp) -> @lifetime(copy 0) @owned NE {
+
+  // NEStoredProp.ne.setter
+  // CHECK-LABEL: sil hidden [transparent] @$s9accessors12NEStoredPropV2neAA2NEVvs : $@convention(method) (@owned NE, @lifetime(copy 0, copy 1) @inout NEStoredProp) -> () {
+
+  // NEStoredProp.ne.modify
+  // CHECK-LABEL: sil hidden [transparent] @$s9accessors12NEStoredPropV2neAA2NEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NEStoredProp) -> @lifetime(copy 0) @yields @inout NE {
+
+  // NEStoredProp.init(ne:)
+  // CHECK-LABEL: sil hidden @$s9accessors12NEStoredPropV2neAcA2NEV_tcfC : $@convention(method) (@owned NE, @thin NEStoredProp.Type) -> @lifetime(copy 0) @owned NEStoredProp {
+}
+
+struct NCNEStoredProp : ~Copyable & ~Escapable {
+  var ncne: NCNE
+  // NCNEStoredProp.ncne.read
+  // CHECK-LABEL: sil hidden [transparent] @$s9accessors14NCNEStoredPropV4ncneAA4NCNEVvr : $@yield_once @convention(method) (@guaranteed NCNEStoredProp) -> @lifetime(copy 0) @yields @guaranteed NCNE {
+
+  // NCNEStoredProp.ncne.setter
+  // CHECK-LABEL: sil hidden [transparent] @$s9accessors14NCNEStoredPropV4ncneAA4NCNEVvs : $@convention(method) (@owned NCNE, @lifetime(copy 0, copy 1) @inout NCNEStoredProp) -> () {
+
+  // NCNEStoredProp.ncne.modify
+  // CHECK-LABEL: sil hidden [transparent] @$s9accessors14NCNEStoredPropV4ncneAA4NCNEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NCNEStoredProp) -> @lifetime(copy 0) @yields @inout NCNE {
+
+  // NCNEStoredProp.init(ncne:)
+  // CHECK-LABEL: sil hidden @$s9accessors14NCNEStoredPropV4ncneAcA4NCNEV_tcfC : $@convention(method) (@owned NCNE, @thin NCNEStoredProp.Type) -> @lifetime(copy 0) @owned NCNEStoredProp {
+}
+
+struct NCNEHolder : ~Copyable, ~Escapable {
+  var p: UnsafeRawPointer
+
+  // this cannot be public in order to synthesize the _read accessor.
+  var ncneBorrow: NCNE {
+    // CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV10ncneBorrowAA4NCNEVvr : $@yield_once @convention(method) (@guaranteed NCNEHolder) -> @lifetime(borrow 0) @yields @guaranteed NCNE {
+    @_lifetime(borrow self)
+    borrowing get {
+      _overrideLifetime(NCNE(p), borrowing: self)
+    }
+    // CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV10ncneBorrowAA4NCNEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NCNEHolder) -> @lifetime(borrow 0) @yields @inout NCNE {
+    @_lifetime(self: copy self)
+    set {
+      p = newValue._p
+    }
+  }
+
+  // this cannot be public in order to synthesize the _read accessor.
+  var ncneCopy: NCNE {
+    // CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV8ncneCopyAA4NCNEVvr : $@yield_once @convention(method) (@guaranteed NCNEHolder) -> @lifetime(copy 0) @yields @guaranteed NCNE {
+    @_lifetime(copy self)
+    borrowing get {
+      _overrideLifetime(NCNE(p), copying: self)
+    }
+    // CHECK-LABEL: sil{{.*}} @$s9accessors10NCNEHolderV8ncneCopyAA4NCNEVvM : $@yield_once @convention(method) (@lifetime(copy 0) @inout NCNEHolder) -> @lifetime(copy 0) @yields @inout NCNE {
+    @_lifetime(self: copy self)
+    set {
+      p = newValue._p
+    }
+  }
+}
