Merge pull request swiftlang#81656 from atrick/lifedep-inout-infer

Allow passing MutableSpan 'inout' without an experimental feature.

Author: atrick

lib/AST/Evaluator.cpp

@@ -127,6 +127,12 @@ void Evaluator::diagnoseCycle(const ActiveRequest &request) {
   for (const auto &step : llvm::reverse(activeRequests)) {
     if (step == request) return;
 
+    // Reporting the lifetime dependence location generates a redundant
+    // diagnostic.
+    if (step.getAs<LifetimeDependenceInfoRequest>()) {
+      continue;
+    }
+
     step.noteCycleStep(diags);
   }
 

lib/AST/LifetimeDependence.cpp

@@ -309,13 +309,21 @@ class LifetimeDependenceChecker {
 
     if (!ctx.LangOpts.hasFeature(Feature::LifetimeDependence)
         && !ctx.SourceMgr.isImportMacroGeneratedLoc(returnLoc)) {
+
+      // Infer inout dependencies without requiring a feature flag. On
+      // returning, 'lifetimeDependencies' contains any inferred
+      // dependencies. This does not issue any diagnostics because any invalid
+      // usage should generate a missing feature flag diagnostic instead.
+      inferInoutParams();
+
       diagnoseMissingResultDependencies(
         diag::lifetime_dependence_feature_required_return.ID);
       diagnoseMissingSelfDependencies(
         diag::lifetime_dependence_feature_required_mutating.ID);
       diagnoseMissingInoutDependencies(
         diag::lifetime_dependence_feature_required_inout.ID);
-      return std::nullopt;
+
+      return currentDependencies();
     }
 
     if (afd->getAttrs().hasAttribute<LifetimeAttr>()) {
@@ -851,22 +859,30 @@ class LifetimeDependenceChecker {
       return;
     }
 
-    // Infer mutating methods.
-    if (hasImplicitSelfParam()) {
-      if (isDiagnosedNonEscapable(dc->getSelfTypeInContext())) {
-        assert(!isInit() && "class initializers have Escapable self");
-        auto *selfDecl = afd->getImplicitSelfDecl();
-        if (selfDecl->isInOut()) {
-          // Mutating methods (excluding initializers)
-          inferMutatingSelf(selfDecl);
-          return;
-        }
-      }
-    }
+    // Infer mutating non-Escapable methods (excluding initializers).
+    inferMutatingSelf();
+
     // Infer inout parameters.
     inferInoutParams();
   }
 
+  /// If the current function is a mutating method and 'self' is non-Escapable,
+  /// return 'self's ParamDecl.
+  bool isMutatingNonEscapableSelf() {
+    if (!hasImplicitSelfParam())
+      return false;
+
+    if (!isDiagnosedNonEscapable(dc->getSelfTypeInContext()))
+      return false;
+
+    assert(!isInit() && "class initializers have Escapable self");
+    auto *selfDecl = afd->getImplicitSelfDecl();
+    if (!selfDecl->isInOut())
+      return false;
+
+    return true;
+  }
+
   // Infer method dependence: result depends on self. This includes _modify.
   void inferNonEscapableResultOnSelf() {
     Type selfTypeInContext = dc->getSelfTypeInContext();
@@ -1069,10 +1085,13 @@ class LifetimeDependenceChecker {
     pushDeps(createDeps(resultIndex).add(*candidateParamIndex,
                                          *candidateLifetimeKind));
   }
-  
+
   // Infer a mutating 'self' dependency when 'self' is non-Escapable and the
   // result is 'void'.
-  void inferMutatingSelf(ParamDecl *selfDecl) {
+  void inferMutatingSelf() {
+    if (!isMutatingNonEscapableSelf()) {
+      return;
+    }
     // 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)) {
@@ -1161,8 +1180,59 @@ class LifetimeDependenceChecker {
   // Infer 'inout' parameter dependency when the only parameter is
   // non-Escapable.
   //
-  // This is needed for most generic Builtin functions.
+  // This supports the common case in which the user of a non-Escapable type,
+  // such as MutableSpan, wants to modify the span's contents without modifying
+  // the span value itself. It should be possible to use MutableSpan this way
+  // without requiring any knowledge of lifetime annotations. The tradeoff is
+  // that it makes authoring non-Escapable types less safe. For example, a
+  // MutableSpan method could update the underlying unsafe pointer and forget to
+  // declare a dependence on the incoming pointer.
+  //
+  // Disallowing other non-Escapable parameters rules out the easy mistake of
+  // programmers attempting to trivially reassign the inout parameter. There's
+  // is no way to rule out the possibility that they derive another
+  // non-Escapable value from an Escapable parameteter. So they can still write
+  // the following and will get a lifetime diagnostic:
+  //
+  //     func reassign(s: inout MutableSpan<Int>, a: [Int]) {
+  //       s = a.mutableSpan
+  //     }
+  //
+  // Do not issue any diagnostics. This inference is triggered even when the
+  // feature is disabled!
   void inferInoutParams() {
+    if (isMutatingNonEscapableSelf()) {
+      return;
+    }
+    std::optional<unsigned> candidateParamIndex;
+    bool hasNonEscapableParameter = false;
+    if (hasImplicitSelfParam()
+        && isDiagnosedNonEscapable(dc->getSelfTypeInContext())) {
+      hasNonEscapableParameter = true;
+    }
+    for (unsigned paramIndex : range(afd->getParameters()->size())) {
+      auto *param = afd->getParameters()->get(paramIndex);
+      if (isDiagnosedNonEscapable(
+            afd->mapTypeIntoContext(param->getInterfaceType()))) {
+        if (param->isInOut()) {
+          if (hasNonEscapableParameter)
+            return;
+          candidateParamIndex = paramIndex;
+          continue;
+        }
+        if (candidateParamIndex)
+          return;
+
+        hasNonEscapableParameter = true;
+      }
+    }
+    if (candidateParamIndex) {
+      pushDeps(createDeps(*candidateParamIndex).add(
+                 *candidateParamIndex, LifetimeDependenceKind::Inherit));
+    }
+  }
+
+  void inferUnambiguousInoutParams() {
     if (afd->getParameters()->size() != 1) {
       return;
     }
@@ -1181,7 +1251,7 @@ class LifetimeDependenceChecker {
 
   void inferBuiltin() {
     // Normal inout parameter inference works for most generic Builtins.
-    inferInoutParams();
+    inferUnambiguousInoutParams();
     if (!lifetimeDependencies.empty()) {
       return;
     }

test/Sema/lifetime_depend_infer.swift

@@ -129,7 +129,11 @@ struct EscapableNonTrivialSelf {
   @lifetime(borrow self)
   func methodNoParamBorrow() -> NEImmortal { NEImmortal() }
 
-  func mutatingMethodNoParam() -> NEImmortal { NEImmortal() }
+  mutating func mutatingMethodNoParam() -> NEImmortal { NEImmortal() }
+
+  func methodInoutNonEscapableParam(_: inout NE) {}
+
+  mutating func mutatingMethodInoutNonEscapableParam(_: inout NE) {}
 
   @lifetime(self)
   mutating func mutatingMethodNoParamLifetime() -> NEImmortal { NEImmortal() }
@@ -276,6 +280,10 @@ func neParamInoutLifetime(ne: inout NE) -> NE { ne }
 
 func neTwoParam(ne: NE, _:Int) -> NE { ne } // expected-error{{a function with a ~Escapable result requires '@lifetime(...)'}}
 
+func voidInoutOneParam(_: inout NE) {} // OK
+
+func voidInoutTwoParams(_: inout NE, _: Int) {} // OK
+
 // =============================================================================
 // Handle Accessors:
 //

test/Sema/lifetime_depend_nofeature.swift

@@ -11,16 +11,37 @@ struct EmptyNonEscapable: ~Escapable {} // expected-error{{an implicit initializ
 // Don't allow non-Escapable return values.
 func neReturn(span: RawSpan) -> RawSpan { span } // expected-error{{a function with a ~Escapable result requires '-enable-experimental-feature LifetimeDependence'}}
 
-func neInout(span: inout RawSpan) {} // expected-error{{a function with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
+func neInout(span: inout RawSpan) {} // OK
+
+func neInoutNEParam(span: inout RawSpan, _: RawSpan) {} // expected-error{{a function with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
 
 struct S {
   func neReturn(span: RawSpan) -> RawSpan { span } // expected-error{{a method with a ~Escapable result requires '-enable-experimental-feature LifetimeDependence'}}
 
-  func neInout(span: inout RawSpan) {} // expected-error{{a method with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
+  func neInout(span: inout RawSpan) {} // OK
+
+  func neInoutNEParam(span: inout RawSpan, _: RawSpan) {} // expected-error{{a method with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
+
+  mutating func mutatingNEInout(span: inout RawSpan) {} // OK
+
+  mutating func mutatingNEInoutParam(span: inout RawSpan, _: RawSpan) {} // expected-error{{a mutating method with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
 }
 
 class C {
   func neReturn(span: RawSpan) -> RawSpan { span } // expected-error{{a method with a ~Escapable result requires '-enable-experimental-feature LifetimeDependence'}}
 
+  func neInout(span: inout RawSpan) {} // OK
+}
+
+extension MutableSpan {
+  func method() {} // OK
+
+  mutating func mutatingMethod() {} // expected-error{{a mutating method with ~Escapable 'self' requires '-enable-experimental-feature LifetimeDependence'}}
+
+  func neReturn(span: RawSpan) -> RawSpan { span } // expected-error{{a method with a ~Escapable result requires '-enable-experimental-feature LifetimeDependence'}}
+
   func neInout(span: inout RawSpan) {} // expected-error{{a method with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
+
+  mutating func mutatingNEInout(span: inout RawSpan) {} // expected-error{{a mutating method with ~Escapable 'self' requires '-enable-experimental-feature LifetimeDependence'}}
+  // expected-error@-1{{a mutating method with a ~Escapable 'inout' parameter requires '-enable-experimental-feature LifetimeDependence'}}
 }

test/decl/protocol/protocols.swift

@@ -111,9 +111,9 @@ struct DoesNotConform : Up {
 
 // Circular protocols
 
+protocol CircleStart : CircleEnd { func circle_start() } // expected-error 2 {{protocol 'CircleStart' refines itself}}
 protocol CircleMiddle : CircleStart { func circle_middle() }
 // expected-note@-1 2 {{protocol 'CircleMiddle' declared here}}
-protocol CircleStart : CircleEnd { func circle_start() } // expected-error 2 {{protocol 'CircleStart' refines itself}}
 protocol CircleEnd : CircleMiddle { func circle_end()} // expected-note 2 {{protocol 'CircleEnd' declared here}}
 
 protocol CircleEntry : CircleTrivial { }