Merge pull request #82505 from atrick/inout-syntax

Fix diagnostics for missing or invalid @_lifetime annotations on inout params

Author: atrick

include/swift/AST/DiagnosticsParse.def

@@ -2147,7 +2147,7 @@ ERROR(expected_lparen_after_lifetime_dependence, PointsToFirstBadToken,
 
 ERROR(expected_identifier_or_index_or_self_after_lifetime_dependence,
       PointsToFirstBadToken,
-      "expected identifier, index or self in lifetime dependence specifier",
+      "expected 'copy', 'borrow', or '&' followed by an identifier, index or 'self' in lifetime dependence specifier",
       ())
 
 ERROR(expected_rparen_after_lifetime_dependence, PointsToFirstBadToken,

include/swift/AST/DiagnosticsSema.def

@@ -8324,7 +8324,7 @@ ERROR(lifetime_dependence_cannot_use_default_escapable_consuming, none,
       "invalid lifetime dependence on an Escapable value with %0 ownership",
       (StringRef))
 ERROR(lifetime_dependence_cannot_use_parsed_borrow_inout, none,
-      "invalid use of borrow dependence on the same inout parameter",
+      "invalid use of inout dependence on the same inout parameter",
       ())
 ERROR(lifetime_dependence_duplicate_target, none,
       "invalid duplicate target lifetime dependencies on function", ())
@@ -8340,18 +8340,18 @@ ERROR(lifetime_dependence_feature_required_return, none,
 ERROR(lifetime_dependence_feature_required_mutating, none,
       "%0 cannot have a ~Escapable 'self'", (StringRef))
 ERROR(lifetime_dependence_feature_required_inout, none,
-      "%0 cannot have a ~Escapable 'inout' parameter %1",
+      "%0 cannot have a ~Escapable 'inout' parameter '%1' in addition to other ~Escapable parameters",
       // this arg list must be compatible with
       // lifetime_dependence_cannot_infer_inout
-      (StringRef, Identifier))
+      (StringRef, StringRef))
 
 ERROR(lifetime_dependence_cannot_infer_return, none,
       "%0 with a ~Escapable result requires '@_lifetime(...)'", (StringRef))
 ERROR(lifetime_dependence_cannot_infer_mutating, none,
       "%0 with a ~Escapable 'self' requires '@_lifetime(self: ...)'", (StringRef))
 ERROR(lifetime_dependence_cannot_infer_inout, none,
       "%0 with a ~Escapable 'inout' parameter requires '@_lifetime(%1: ...)'",
-      (StringRef, Identifier))
+      (StringRef, StringRef))
 
 //------------------------------------------------------------------------------
 // MARK: Lifetime Dependence Inference - refinements to the requirements above
@@ -8374,6 +8374,9 @@ ERROR(lifetime_dependence_cannot_infer_kind, none,
 ERROR(lifetime_dependence_cannot_infer_scope_ownership, none,
       "cannot borrow the lifetime of '%0', which has consuming ownership on %1",
       (StringRef, StringRef))
+NOTE(lifetime_dependence_cannot_infer_inout_suggest, none,
+     "use '@_lifetime(%0: copy %0) to forward the inout dependency",
+     (StringRef))
 
 //------------------------------------------------------------------------------
 // MARK: Lifetime Dependence Experimental Inference

lib/AST/LifetimeDependence.cpp

@@ -554,7 +554,13 @@ class LifetimeDependenceChecker {
                         })) {
         ctx.Diags.diagnose(param->getLoc(), diagID,
                            {StringRef(diagnosticQualifier()),
-                            param->getName()});
+                            param->getName().str()});
+        if (diagID == diag::lifetime_dependence_cannot_infer_inout.ID) {
+          ctx.Diags.diagnose(
+            param->getLoc(),
+            diag::lifetime_dependence_cannot_infer_inout_suggest,
+            param->getName().str());
+        }
       }
     }
   }
@@ -904,16 +910,21 @@ class LifetimeDependenceChecker {
       if (!paramDeclAndIndex.has_value()) {
         return std::nullopt;
       }
-      auto lifetimeKind =
-        getDependenceKindFromDescriptor(source, paramDeclAndIndex->first);
+      auto *param = paramDeclAndIndex->first;
+      unsigned sourceIndex = paramDeclAndIndex->second;
+      auto lifetimeKind = getDependenceKindFromDescriptor(source, param);
       if (!lifetimeKind.has_value()) {
         return std::nullopt;
       }
-      unsigned sourceIndex = paramDeclAndIndex->second;
       if (lifetimeKind == LifetimeDependenceKind::Scope
+          && param->isInOut()
           && sourceIndex == targetIndex) {
         diagnose(source.getLoc(),
                  diag::lifetime_dependence_cannot_use_parsed_borrow_inout);
+        ctx.Diags.diagnose(source.getLoc(),
+                           diag::lifetime_dependence_cannot_infer_inout_suggest,
+                           param->getName().str());
+
         return std::nullopt;
       }
       bool hasError =

lib/Parse/ParseDecl.cpp

@@ -5014,6 +5014,7 @@ ParserResult<LifetimeEntry> Parser::parseLifetimeEntry(SourceLoc loc) {
   SmallVector<LifetimeDescriptor> sources;
   SourceLoc rParenLoc;
   bool foundParamId = false;
+  bool invalidSourceDescriptor = false;
   status = parseList(
       tok::r_paren, lParenLoc, rParenLoc, /*AllowSepAfterLast=*/false,
       diag::expected_rparen_after_lifetime_dependence, [&]() -> ParserStatus {
@@ -5030,13 +5031,18 @@ ParserResult<LifetimeEntry> Parser::parseLifetimeEntry(SourceLoc loc) {
         auto sourceDescriptor =
             parseLifetimeDescriptor(*this, lifetimeDependenceKind);
         if (!sourceDescriptor) {
+          invalidSourceDescriptor = true;
           listStatus.setIsParseError();
           return listStatus;
         }
         sources.push_back(*sourceDescriptor);
         return listStatus;
       });
 
+  if (invalidSourceDescriptor) {
+    status.setIsParseError();
+    return status;
+  }
   if (!foundParamId) {
     diagnose(
         Tok,

test/Parse/lifetime_attr.swift

@@ -24,7 +24,7 @@ func testMissingLParenError(_ ne: NE) -> NE { // expected-error{{cannot infer th
   ne
 }
 
-@_lifetime() // expected-error{{expected identifier, index or self in lifetime dependence specifier}}
+@_lifetime() // expected-error{{expected 'copy', 'borrow', or '&' followed by an identifier, index or 'self' in lifetime dependence specifier}}
 func testMissingDependence(_ ne: NE) -> NE { // expected-error{{cannot infer the lifetime dependence scope on a function with a ~Escapable parameter, specify '@_lifetime(borrow ne)' or '@_lifetime(copy ne)'}}
   ne
 }

test/Sema/lifetime_attr_nofeature.swift

@@ -8,3 +8,9 @@ struct NE : ~Escapable { // expected-error{{an implicit initializer cannot retur
 func derive(_ ne: NE) -> NE { // expected-error{{a function cannot return a ~Escapable result}}
   ne
 }
+
+func f_inout_infer(a: inout MutableRawSpan) {}
+
+func f_inout_no_infer(a: inout MutableRawSpan, b: RawSpan) {}
+// expected-error @-1{{a function cannot have a ~Escapable 'inout' parameter 'a' in addition to other ~Escapable parameters}}
+

test/Sema/lifetime_depend_infer.swift

@@ -15,6 +15,8 @@ struct NEImmortal: ~Escapable {
   init() {}
 }
 
+struct MutNE: ~Copyable & ~Escapable {}
+
 // =============================================================================
 // Handle non-Escapable results with 'self'
 // =============================================================================
@@ -563,7 +565,8 @@ struct NonEscapableMutableSelf: ~Escapable {
   @_lifetime(self: copy self) // OK
   mutating func mutatingMethodNoParamCopy() {}
 
-  @_lifetime(self: &self) // expected-error{{invalid use of borrow dependence on the same inout parameter}}
+  @_lifetime(self: &self) // expected-error{{invalid use of inout dependence on the same inout parameter}}
+                          // expected-note @-1{{use '@_lifetime(self: copy self) to forward the inout dependency}}
   mutating func mutatingMethodNoParamBorrow() {}
 
   mutating func mutatingMethodOneParam(_: NE) {} // expected-error{{a mutating method with a ~Escapable 'self' requires '@_lifetime(self: ...)'}}
@@ -577,3 +580,24 @@ struct NonEscapableMutableSelf: ~Escapable {
   @_lifetime(&self)
   mutating func mutatingMethodOneParamBorrow(_: NE) {}
 }
+
+// =============================================================================
+// Handle common mistakes with inout parameter annotations
+// =============================================================================
+
+// Unable to infer an 'inout' dependency. Provide valid guidance.
+//
+func f_inout_no_infer(a: inout MutNE, b: NE) {}
+// expected-error @-1{{a function with a ~Escapable 'inout' parameter requires '@_lifetime(a: ...)'}}
+// expected-note  @-2{{use '@_lifetime(a: copy a) to forward the inout dependency}}
+
+// Invalid keyword for the dependence kind.
+//
+@_lifetime(a: inout a) // expected-error{{expected 'copy', 'borrow', or '&' followed by an identifier, index or 'self' in lifetime dependence specifier}}
+func f_inout_bad_keyword(a: inout MutableRawSpan) {}
+
+// Don't allow a useless borrow dependency on an inout param--it is misleading.
+//
+@_lifetime(a: &a) // expected-error{{invalid use of inout dependence on the same inout parameter}}
+                  // expected-note @-1{{use '@_lifetime(a: copy a) to forward the inout dependency}}
+func f_inout_useless(a: inout MutableRawSpan) {}