Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
23 changes: 19 additions & 4 deletions lib/SILOptimizer/Mandatory/DiagnoseInvalidEscapingCaptures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand All @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions test/SILOptimizer/OSLogCompilerDiagnosticsTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
58 changes: 58 additions & 0 deletions test/SILOptimizer/invalid_escaping_captures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
}
}