Merge pull request swiftlang#82189 from atrick/lifedep-trivial-inout

Disallow @_lifetime(borrow) for trivial 'inout' arguments

Author: atrick

include/swift/AST/DiagnosticsSema.def

@@ -8319,11 +8319,14 @@ ERROR(lifetime_dependence_immortal_alone, none,
       "cannot specify any other dependence source along with immortal", ())
 ERROR(lifetime_dependence_invalid_inherit_escapable_type, none,
       "cannot copy the lifetime of an Escapable type, use "
-      "'@_lifetime(%1%0)' instead",
+      "'@_lifetime(%0%1)' instead",
       (StringRef, StringRef))
-ERROR(lifetime_dependence_cannot_use_parsed_borrow_consuming, none,
+ERROR(lifetime_dependence_parsed_borrow_with_ownership, none,
       "invalid use of %0 dependence with %1 ownership",
       (StringRef, StringRef))
+NOTE(lifetime_dependence_parsed_borrow_with_ownership_fix, none,
+     "use '@_lifetime(%0%1)' instead",
+     (StringRef, StringRef))
 ERROR(lifetime_dependence_cannot_use_default_escapable_consuming, none,
       "invalid lifetime dependence on an Escapable value with %0 ownership",
       (StringRef))

lib/AST/LifetimeDependence.cpp

@@ -504,20 +504,18 @@ class LifetimeDependenceChecker {
   }
 
   bool isCompatibleWithOwnership(ParsedLifetimeDependenceKind kind, Type type,
-                                 ValueOwnership ownership,
+                                 ValueOwnership loweredOwnership,
                                  bool isInterfaceFile = false) const {
     if (kind == ParsedLifetimeDependenceKind::Inherit) {
       return true;
     }
-    // Lifetime dependence always propagates through temporary BitwiseCopyable
-    // values, even if the dependence is scoped.
-    if (isBitwiseCopyable(type, ctx)) {
-      return true;
-    }
-    auto loweredOwnership = ownership != ValueOwnership::Default
-      ? ownership : getLoweredOwnership(afd);
-
     if (kind == ParsedLifetimeDependenceKind::Borrow) {
+      // An owned/consumed BitwiseCopyable value can be effectively borrowed
+      // because its lifetime can be indefinitely extended.
+      if (loweredOwnership == ValueOwnership::Owned
+          && isBitwiseCopyable(type, ctx)) {
+        return true;
+      }
       if (isInterfaceFile) {
         return loweredOwnership == ValueOwnership::Shared ||
                loweredOwnership == ValueOwnership::InOut;
@@ -651,37 +649,55 @@ class LifetimeDependenceChecker {
 
     case ParsedLifetimeDependenceKind::Borrow: LLVM_FALLTHROUGH;
     case ParsedLifetimeDependenceKind::Inout: {
-    // @lifetime(borrow x) is valid only for borrowing parameters.
-    // @lifetime(inout x) is valid only for inout parameters.
-    if (!isCompatibleWithOwnership(parsedLifetimeKind, type, loweredOwnership,
-                                   isInterfaceFile())) {
+      // @lifetime(borrow x) is valid only for borrowing parameters.
+      // @lifetime(&x) is valid only for inout parameters.
+      auto loweredOwnership = ownership != ValueOwnership::Default
+        ? ownership : getLoweredOwnership(afd);
+      if (isCompatibleWithOwnership(parsedLifetimeKind, type, loweredOwnership,
+                                    isInterfaceFile())) {
+        return LifetimeDependenceKind::Scope;
+      }
       diagnose(loc,
-               diag::lifetime_dependence_cannot_use_parsed_borrow_consuming,
+               diag::lifetime_dependence_parsed_borrow_with_ownership,
                getNameForParsedLifetimeDependenceKind(parsedLifetimeKind),
                getOwnershipSpelling(loweredOwnership));
+      switch (loweredOwnership) {
+      case ValueOwnership::Shared:
+        diagnose(loc,
+                 diag::lifetime_dependence_parsed_borrow_with_ownership_fix,
+                 "borrow ", descriptor.getString());
+        break;
+      case ValueOwnership::InOut:
+        diagnose(loc,
+                 diag::lifetime_dependence_parsed_borrow_with_ownership_fix,
+                 "&", descriptor.getString());
+        break;
+      case ValueOwnership::Owned:
+      case ValueOwnership::Default:
+        break;
+      }
       return std::nullopt;
     }
-    return LifetimeDependenceKind::Scope;
-  }
-  case ParsedLifetimeDependenceKind::Inherit:
-    // @lifetime(copy x) is only invalid for Escapable types.
-    if (type->isEscapable()) {
-      if (loweredOwnership == ValueOwnership::Shared) {
-        diagnose(loc, diag::lifetime_dependence_invalid_inherit_escapable_type,
-                 descriptor.getString(), "borrow ");
-      } else if (loweredOwnership == ValueOwnership::InOut) {
-        diagnose(loc, diag::lifetime_dependence_invalid_inherit_escapable_type,
-                 descriptor.getString(), "&");
-      } else {
-        diagnose(
+    case ParsedLifetimeDependenceKind::Inherit: {
+      // @lifetime(copy x) is only invalid for Escapable types.
+      if (type->isEscapable()) {
+        if (loweredOwnership == ValueOwnership::Shared) {
+          diagnose(loc, diag::lifetime_dependence_invalid_inherit_escapable_type,
+                   "borrow ", descriptor.getString());
+        } else if (loweredOwnership == ValueOwnership::InOut) {
+          diagnose(loc, diag::lifetime_dependence_invalid_inherit_escapable_type,
+                   "&", descriptor.getString());
+        } else {
+          diagnose(
             loc,
             diag::lifetime_dependence_cannot_use_default_escapable_consuming,
             getOwnershipSpelling(loweredOwnership));
+        }
+        return std::nullopt;
       }
-      return std::nullopt;
+      return LifetimeDependenceKind::Inherit;
+    }
     }
-    return LifetimeDependenceKind::Inherit;
-  }
   }
 
   // Finds the ParamDecl* and its index from a LifetimeDescriptor

test/SIL/implicit_lifetime_dependence.swift

@@ -89,7 +89,7 @@ struct Wrapper : ~Escapable {
       yield _view
     }
 // CHECK: sil hidden @$s28implicit_lifetime_dependence7WrapperV4viewAA10BufferViewVvM : $@yield_once @convention(method) (@inout Wrapper) -> @lifetime(borrow 0) @yields @inout BufferView {
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     _modify {
       yield &_view
     }

test/SILOptimizer/lifetime_dependence/coroutine.swift

@@ -40,7 +40,7 @@ struct Wrapper : ~Escapable {
     _read {
       yield _view
     }
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     _modify {
       yield &_view
     }

test/SILOptimizer/lifetime_dependence/dependence_insertion.swift

@@ -109,7 +109,7 @@ func pointerDepends(on x: Builtin.RawPointer) -> NERawPointer {
 // CHECK: [[NE:%[0-9]+]] = apply %{{.*}}([[PTR]]) : $@convention(thin) (Builtin.RawPointer) -> @lifetime(borrow 0) @owned NERawPointer
 // CHECK: mark_dependence [unresolved] [[NE]] on %0
 // CHECK-LABEL: } // end sil function '$s4test0A22BuiltinAddressOfBorrow6holderAA12NERawPointerVAA9IntHolderVz_tF'
-@_lifetime(borrow holder)
+@_lifetime(&holder)
 func testBuiltinAddressOfBorrow(holder: inout IntHolder) -> NERawPointer {
   pointerDepends(on: Builtin.addressOfBorrow(holder))
 }

test/SILOptimizer/lifetime_dependence/lifetime_dependence_inherit.swift

@@ -58,7 +58,7 @@ struct NEBV : ~Escapable {
     _read {
       yield bv
     }
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     _modify {
       yield &bv
     }

test/SILOptimizer/lifetime_dependence/lifetime_dependence_scope_fixup.swift

@@ -224,7 +224,7 @@ struct Wrapper : ~Escapable {
     _read {
       yield _view
     }
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     _modify {
       yield &_view
     }

test/SILOptimizer/lifetime_dependence/semantics.swift

@@ -67,7 +67,7 @@ struct MutableSpan<T>: ~Escapable, ~Copyable {
   let base: UnsafeMutablePointer<T>
   let count: Int
 
-  @_lifetime(&base)
+  @_lifetime(borrow base)
   init(base: UnsafeMutablePointer<T>, count: Int) {
     self.base = base
     self.count = count
@@ -404,7 +404,7 @@ public func testTrivialLocalDeadEnd(p: UnsafePointer<Int>) {
 
 // Test dependence on a borrow of a trivial inout. The access scope can be ignored since we don't care about the
 // in-memory value.
-@_lifetime(borrow p)
+@_lifetime(&p)
 public func testTrivialInoutBorrow(p: inout UnsafePointer<Int>) -> Span<Int> {
   return Span(base: p, count: 1)
 }
@@ -591,7 +591,7 @@ func testNonAddressable(arg: Holder) -> Span<Int> {
   // expected-note  @-3{{it depends on the lifetime of argument 'arg'}}
 } // expected-note  {{this use causes the lifetime-dependent value to escape}}
 
-@_lifetime(borrow arg)
+@_lifetime(&arg)
 func test(arg: inout AddressableInt) -> Span<Int> {
   arg.span()
 }

test/Sema/lifetime_attr.swift

@@ -30,7 +30,36 @@ func invalidDuplicateLifetimeDependence1(_ ne: borrowing NE) -> NE {
 class Klass {}
 
 @_lifetime(borrow x) // expected-error{{invalid use of borrow dependence with consuming ownership}}
-func invalidDependence(_ x: consuming Klass) -> NE {
+func invalidDependenceConsumeKlass(_ x: consuming Klass) -> NE {
+  NE()
+}
+
+@_lifetime(&x) // expected-error{{invalid use of & dependence with borrowing ownership}}
+               // expected-note @-1{{use '@_lifetime(borrow x)' instead}}
+func invalidDependenceBorrowKlass(_ x: borrowing Klass) -> NE {
+  NE()
+}
+
+@_lifetime(borrow x) // expected-error{{invalid use of borrow dependence with inout ownership}}
+                     // expected-note @-1{{use '@_lifetime(&x)' instead}}
+func invalidDependenceInoutKlass(_ x: inout Klass) -> NE {
+  NE()
+}
+
+@_lifetime(borrow x) // OK
+func invalidDependenceConsumeInt(_ x: consuming Int) -> NE {
+  NE()
+}
+
+@_lifetime(&x) // expected-error{{invalid use of & dependence with borrowing ownership}}
+               // expected-note @-1{{use '@_lifetime(borrow x)' instead}}
+func invalidDependenceBorrowInt(_ x: borrowing Int) -> NE {
+  NE()
+}
+
+@_lifetime(borrow x) // expected-error{{invalid use of borrow dependence with inout ownership}}
+                     // expected-note @-1{{use '@_lifetime(&x)' instead}}
+func invalidDependenceInoutInt(_ x: inout Int) -> NE {
   NE()
 }
 

test/Sema/lifetime_depend_infer.swift

@@ -40,7 +40,7 @@ struct NonEscapableSelf: ~Escapable {
   @_lifetime(copy self) // OK
   mutating func mutatingMethodNoParamCopy() -> NonEscapableSelf { self }
 
-  @_lifetime(borrow self) // OK
+  @_lifetime(&self) // OK
   mutating func mutatingMethodNoParamBorrow() -> NonEscapableSelf { self }
 
   func methodOneParam(_: Int) -> NonEscapableSelf { self } // expected-error{{a method with a ~Escapable result requires '@_lifetime(...)'}}
@@ -63,7 +63,7 @@ struct NonEscapableSelf: ~Escapable {
   @_lifetime(copy self) // OK
   mutating func mutatingMethodOneParamCopy(_: Int) -> NonEscapableSelf { self }
 
-  @_lifetime(borrow self) // OK
+  @_lifetime(&self) // OK
   mutating func mutatingMethodOneParamBorrow(_: Int) -> NonEscapableSelf { self }
 }
 
@@ -87,7 +87,7 @@ struct EscapableTrivialSelf {
   @_lifetime(copy self) // expected-error{{cannot copy the lifetime of an Escapable type, use '@_lifetime(&self)' instead}}
   mutating func mutatingMethodNoParamCopy() -> NEImmortal { NEImmortal() }
 
-  @_lifetime(borrow self)
+  @_lifetime(&self)
   mutating func mutatingMethodNoParamBorrow() -> NEImmortal { NEImmortal() }
 
   func methodOneParam(_: Int) -> NEImmortal { NEImmortal() } // expected-error{{a method with a ~Escapable result requires '@_lifetime(...)'}}
@@ -109,7 +109,7 @@ struct EscapableTrivialSelf {
   @_lifetime(copy self) // expected-error{{cannot copy the lifetime of an Escapable type, use '@_lifetime(&self)' instead}}
   mutating func mutatingMethodOneParamCopy(_: Int) -> NEImmortal { NEImmortal() }
 
-  @_lifetime(borrow self)
+  @_lifetime(&self)
   mutating func mutatingMethodOneParamBorrow(_: Int) -> NEImmortal { NEImmortal() }
 }
 
@@ -341,7 +341,7 @@ struct NonescapableSelfAccessors: ~Escapable {
       yield ne
     }
 
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     _modify {
       yield &ne
     }
@@ -401,7 +401,7 @@ struct NonescapableSelfAccessors: ~Escapable {
       ne
     }
 
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     set {
       ne = newValue
     }
@@ -413,7 +413,7 @@ struct NonescapableSelfAccessors: ~Escapable {
       yield ne
     }
 
-    @_lifetime(borrow self)
+    @_lifetime(&self)
     _modify {
       yield &ne
     }
@@ -533,7 +533,7 @@ struct NonEscapableMutableSelf: ~Escapable {
   @_lifetime(self: copy self) // OK
   mutating func mutatingMethodNoParamCopy() {}
 
-  @_lifetime(self: borrow self) // expected-error{{invalid use of borrow dependence on the same inout parameter}}
+  @_lifetime(self: &self) // expected-error{{invalid use of borrow dependence on the same inout parameter}}
   mutating func mutatingMethodNoParamBorrow() {}
 
   mutating func mutatingMethodOneParam(_: NE) {} // expected-error{{a mutating method with a ~Escapable 'self' requires '@_lifetime(self: ...)'}}
@@ -544,6 +544,6 @@ struct NonEscapableMutableSelf: ~Escapable {
   @_lifetime(copy self) // OK
   mutating func mutatingMethodOneParamCopy(_: NE) {}
 
-  @_lifetime(borrow self)
+  @_lifetime(&self)
   mutating func mutatingMethodOneParamBorrow(_: NE) {}
 }