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

    @_lifetime(borrow holder) // ERROR
    func test(holder: inout Holder) -> NE

Fixes rdar://153040843 ([nonescapable] disallow @_lifetime(borrow)
for trivial 'inout' arguments)

(cherry picked from commit a389254)

Author: atrick

include/swift/AST/DiagnosticsSema.def

@@ -8232,11 +8232,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;
@@ -653,26 +651,42 @@ class LifetimeDependenceChecker {
     case ParsedLifetimeDependenceKind::Inout: {
       // @lifetime(borrow x) is valid only for borrowing parameters.
       // @lifetime(&x) is valid only for inout parameters.
-      if (!isCompatibleWithOwnership(parsedLifetimeKind, type, loweredOwnership,
-                                     isInterfaceFile())) {
-        diagnose(
-          loc,
-          diag::lifetime_dependence_cannot_use_parsed_borrow_consuming,
-          getNameForParsedLifetimeDependenceKind(parsedLifetimeKind),
-          getOwnershipSpelling(loweredOwnership));
-        return std::nullopt;
+      auto loweredOwnership = ownership != ValueOwnership::Default
+        ? ownership : getLoweredOwnership(afd);
+      if (isCompatibleWithOwnership(parsedLifetimeKind, type, loweredOwnership,
+                                    isInterfaceFile())) {
+        return LifetimeDependenceKind::Scope;
       }
-      return LifetimeDependenceKind::Scope;
+      diagnose(loc,
+               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;
     }
     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 ");
+                   "borrow ", descriptor.getString());
         } else if (loweredOwnership == ValueOwnership::InOut) {
           diagnose(loc, diag::lifetime_dependence_invalid_inherit_escapable_type,
-                   descriptor.getString(), "&");
+                   "&", descriptor.getString());
         } else {
           diagnose(
             loc,

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()
 }