Merge pull request #31139 from ravikandhadai/autoclosure-capture-diagnostics

[SIL Diagnostics] Improve diagnostics for capture of inout values in escaping autoclosures.

Author: ravikandhadai

include/swift/AST/DiagnosticsSIL.def

@@ -108,7 +108,7 @@ ERROR(capture_before_declaration_defer,none,
 NOTE(captured_value_declared_here,none,
      "captured value declared here", ())
 
-#define SELECT_ESCAPING_CLOSURE_KIND "escaping %select{local function|closure}0"
+#define SELECT_ESCAPING_CLOSURE_KIND "escaping %select{local function|closure|autoclosure}0"
 
 // Invalid escaping capture diagnostics.
 ERROR(escaping_inout_capture,none,
@@ -133,6 +133,10 @@ ERROR(escaping_noescape_var_capture,none,
 
 NOTE(value_captured_here,none,"captured here", ())
 
+NOTE(copy_inout_captured_by_autoclosure,none, "pass a copy of %0", (Identifier))
+
+NOTE(copy_self_captured_by_autoclosure,none, "pass a copy of 'self'", ())
+
 #undef SELECT_ESCAPING_CLOSURE_KIND
 
 NOTE(value_captured_transitively,none,

lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp

@@ -17,6 +17,7 @@
 
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/DiagnosticsSIL.h"
+#include "swift/AST/Expr.h"
 #include "swift/AST/Types.h"
 #include "swift/SIL/ApplySite.h"
 #include "swift/SIL/InstructionUtils.h"
@@ -332,17 +333,22 @@ static void checkPartialApply(ASTContext &Context, DeclContext *DC,
   // Should match SELECT_ESCAPING_CLOSURE_KIND in DiagnosticsSIL.def.
   enum {
     EscapingLocalFunction,
-    EscapingClosure
+    EscapingClosure,
+    EscapingAutoClosure,
   } functionKind = EscapingClosure;
 
   if (auto *F = PAI->getReferencedFunctionOrNull()) {
     if (auto loc = F->getLocation()) {
-      if (loc.isASTNode<FuncDecl>())
+      if (loc.isASTNode<FuncDecl>()) {
         functionKind = EscapingLocalFunction;
+      } else if (loc.isASTNode<AutoClosureExpr>()) {
+        functionKind = EscapingAutoClosure;
+      }
     }
   }
   // First, diagnose the inout captures, if any.
   for (auto inoutCapture : inoutCaptures) {
+    Optional<Identifier> paramName = None;
     if (isUseOfSelfInInitializer(inoutCapture)) {
       diagnose(Context, PAI->getLoc(), diag::escaping_mutable_self_capture,
                functionKind);
@@ -352,14 +358,23 @@ static void checkPartialApply(ASTContext &Context, DeclContext *DC,
         diagnose(Context, PAI->getLoc(), diag::escaping_mutable_self_capture,
                  functionKind);
       else {
+        paramName = param->getName();
         diagnose(Context, PAI->getLoc(), diag::escaping_inout_capture,
                  functionKind, param->getName());
         diagnose(Context, param->getLoc(), diag::inout_param_defined_here,
                  param->getName());
       }
     }
-
-    diagnoseCaptureLoc(Context, DC, PAI, inoutCapture);
+    if (functionKind != EscapingAutoClosure) {
+      diagnoseCaptureLoc(Context, DC, PAI, inoutCapture);
+      continue;
+    }
+    // For an autoclosure capture, present a way to fix the problem.
+    if (paramName)
+      diagnose(Context, PAI->getLoc(), diag::copy_inout_captured_by_autoclosure,
+               paramName.getValue());
+    else
+      diagnose(Context, PAI->getLoc(), diag::copy_self_captured_by_autoclosure);
   }
 
   // Finally, diagnose captures of values with noescape type.

test/SILOptimizer/OSLogCompilerDiagnosticsTest.swift

@@ -58,10 +58,10 @@ func testUnreachableLogCall(c: Color)  {
 
 // Passing InOut values to the logger should not crash the compiler.
 func foo(_ mutableValue: inout String) {
+  // expected-note@-1 {{parameter 'mutableValue' is declared 'inout'}}
    _osLogTestHelper("FMFLabelledLocation: initialized with coder \(mutableValue)")
-    // expected-error@-1 {{escaping closure captures 'inout' parameter 'mutableValue'}}
-    // expected-note@-3 {{parameter 'mutableValue' is declared 'inout'}}
-    // expected-note@-3 {{captured here}}
+    // expected-error@-1 {{escaping autoclosure captures 'inout' parameter 'mutableValue'}}
+    // expected-note@-2 {{pass a copy of 'mutableValue'}}
 }
 
 // This is an extension used only for testing a diagnostic that doesn't arise

test/SILOptimizer/invalid_escaping_captures.swift

@@ -189,3 +189,61 @@ public struct SelfEscapeFromInit {
 
   public mutating func handler() {}
 }
+
+func autoclosureTakesEscaping(_ x: @escaping @autoclosure () ->Int) {}
+
+// Test that captures of escaping autoclosure are diagnosed correctly.
+func badCaptureInAutoclosure(x: inout Int) {
+    // expected-note@-1 {{parameter 'x' is declared 'inout'}}
+    // expected-note@-2 {{parameter 'x' is declared 'inout'}}
+
+  autoclosureTakesEscaping(x)
+    // expected-error@-1 {{escaping autoclosure captures 'inout' parameter 'x'}}
+    // expected-note@-2 {{pass a copy of 'x'}}
+
+  autoclosureTakesEscaping((x + 1) - 100)
+    // expected-error@-1 {{escaping autoclosure captures 'inout' parameter 'x'}}
+    // expected-note@-2 {{pass a copy of 'x'}}
+}
+
+// Test that transitive captures in autoclosures are diagnosed correctly.
+func badTransitiveCaptureInClosures(x: inout Int) -> ((Int) -> Void) {
+    // expected-note@-1 {{parameter 'x' is declared 'inout'}}
+    // expected-note@-2 {{parameter 'x' is declared 'inout'}}
+    // expected-note@-3 {{parameter 'x' is declared 'inout'}}
+
+  // Test capture of x by an autoclosure within a non-escaping closure.
+  let _ = { (y: Int) in
+    autoclosureTakesEscaping(x + y)
+      // expected-error@-1 {{escaping autoclosure captures 'inout' parameter 'x'}}
+      // expected-note@-2 {{pass a copy of 'x'}}
+  }
+
+  // Test capture of x by an autoclosure within an escaping closure.
+  let escapingClosure = { (y: Int) in
+      // expected-error@-1 {{escaping closure captures 'inout' parameter 'x'}}
+
+    autoclosureTakesEscaping(x + y)
+      // expected-note@-1 {{captured indirectly by this call}}
+      // expected-note@-2 {{captured here}}
+
+      // expected-error@-4 {{escaping autoclosure captures 'inout' parameter 'x'}}
+      // expected-note@-5 {{pass a copy of 'x'}}
+  }
+  return escapingClosure
+}
+
+// Test that captures of mutating 'self' in escaping autoclosures are diagnosed correctly.
+struct S {
+  var i = 0
+  init() {
+    autoclosureTakesEscaping(i)
+      // expected-error@-1 {{escaping autoclosure captures mutating 'self' parameter}}
+      // expected-note@-2 {{pass a copy of 'self'}}
+  }
+  mutating func method() {
+    autoclosureTakesEscaping(i)
+      // expected-error@-1 {{escaping autoclosure captures mutating 'self' parameter}}
+      // expected-note@-2 {{pass a copy of 'self'}}
+  }
+}