Skip to content

[Sema] Improving implicit closure capture diagnostic wording #58456

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
9 changes: 9 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,9 @@ NOTE(non_sendable_nominal,none,
NOTE(add_nominal_sendable_conformance,none,
"consider making %0 %1 conform to the 'Sendable' protocol",
(DescriptiveDeclKind, DeclName))
NOTE(add_generic_parameter_sendable_conformance,none,
"consider making generic parameter %0 conform to the 'Sendable' protocol",
(Type))
REMARK(add_predates_concurrency_import,none,
"add '@preconcurrency' to %select{suppress|treat}0 "
"'Sendable'-related %select{warnings|errors}0 from module %1"
Expand Down Expand Up @@ -4570,6 +4573,12 @@ ERROR(concurrent_access_of_inout_param,none,
ERROR(non_sendable_capture,none,
"capture of %1 with non-sendable type %0 in a `@Sendable` closure",
(Type, DeclName))
ERROR(implicit_async_let_non_sendable_capture,none,
"capture of %1 with non-sendable type %0 in 'async let' binding",
(Type, DeclName))
ERROR(implicit_non_sendable_capture,none,
"implicit capture of %1 requires that %0 conforms to `Sendable`",
(Type, DeclName))

NOTE(actor_isolated_sync_func,none,
"calls to %0 %1 from outside of its actor context are "
Expand Down
78 changes: 64 additions & 14 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -569,18 +569,27 @@ static void addSendableFixIt(
const NominalTypeDecl *nominal, InFlightDiagnostic &diag, bool unchecked) {
if (nominal->getInherited().empty()) {
SourceLoc fixItLoc = nominal->getBraces().Start;
if (unchecked)
diag.fixItInsert(fixItLoc, ": @unchecked Sendable");
else
diag.fixItInsert(fixItLoc, ": Sendable");
diag.fixItInsert(fixItLoc,
unchecked ? ": @unchecked Sendable" : ": Sendable");
} else {
ASTContext &ctx = nominal->getASTContext();
SourceLoc fixItLoc = nominal->getInherited().back().getSourceRange().End;
fixItLoc = Lexer::getLocForEndOfToken(ctx.SourceMgr, fixItLoc);
if (unchecked)
diag.fixItInsert(fixItLoc, ", @unchecked Sendable");
else
diag.fixItInsert(fixItLoc, ", Sendable");
auto fixItLoc = nominal->getInherited().back().getLoc();
diag.fixItInsertAfter(fixItLoc,
unchecked ? ", @unchecked Sendable" : ", Sendable");
}
}

/// Add Fix-It text for the given generic param declaration type to adopt
/// Sendable.
static void addSendableFixIt(const GenericTypeParamDecl *genericArgument,
InFlightDiagnostic &diag, bool unchecked) {
if (genericArgument->getInherited().empty()) {
auto fixItLoc = genericArgument->getLoc();
diag.fixItInsertAfter(fixItLoc,
unchecked ? ": @unchecked Sendable" : ": Sendable");
} else {
auto fixItLoc = genericArgument->getInherited().back().getLoc();
diag.fixItInsertAfter(fixItLoc,
unchecked ? ", @unchecked Sendable" : ", Sendable");
}
}

Expand Down Expand Up @@ -874,6 +883,18 @@ static bool diagnoseSingleNonSendableType(
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());
} else if (auto genericArchetype = type->getAs<ArchetypeType>()) {
auto interfaceType = genericArchetype->getInterfaceType();
if (auto genericParamType =
interfaceType->getAs<GenericTypeParamType>()) {
auto *genericParamTypeDecl = genericParamType->getDecl();
if (genericParamTypeDecl &&
genericParamTypeDecl->getModuleContext() == module) {
auto diag = genericParamTypeDecl->diagnose(
diag::add_generic_parameter_sendable_conformance, type);
addSendableFixIt(genericParamTypeDecl, diag, /*unchecked=*/false);
}
}
}

return false;
Expand Down Expand Up @@ -1527,6 +1548,7 @@ namespace {
SmallVector<const DeclContext *, 4> contextStack;
SmallVector<ApplyExpr*, 4> applyStack;
SmallVector<std::pair<OpaqueValueExpr *, Expr *>, 4> opaqueValues;
SmallVector<const PatternBindingDecl *, 2> patternBindingStack;

/// Keeps track of the capture context of variables that have been
/// explicitly captured in closures.
Expand All @@ -1539,6 +1561,10 @@ namespace {
using MutableVarParent
= llvm::PointerUnion<InOutExpr *, LoadExpr *, AssignExpr *>;

const PatternBindingDecl *getTopPatternBindingDecl() const {
return patternBindingStack.empty() ? nullptr : patternBindingStack.back();
}

/// Mapping from mutable variable reference exprs, or inout expressions,
/// to the parent expression, when that parent is either a load or
/// an inout expr.
Expand Down Expand Up @@ -1694,9 +1720,24 @@ namespace {
Type type = getDeclContext()
->mapTypeIntoContext(decl->getInterfaceType())
->getReferenceStorageReferent();
diagnoseNonSendableTypes(
type, getDeclContext(), capture.getLoc(),
diag::non_sendable_capture, decl->getName());

if (closure->isImplicit()) {
auto *patternBindingDecl = getTopPatternBindingDecl();
if (patternBindingDecl && patternBindingDecl->isAsyncLet()) {
diagnoseNonSendableTypes(
type, getDeclContext(), capture.getLoc(),
diag::implicit_async_let_non_sendable_capture, decl->getName());
} else {
// Fallback to a generic implicit capture missing sendable
// conformance diagnostic.
diagnoseNonSendableTypes(type, getDeclContext(), capture.getLoc(),
diag::implicit_non_sendable_capture,
decl->getName());
}
} else {
diagnoseNonSendableTypes(type, getDeclContext(), capture.getLoc(),
diag::non_sendable_capture, decl->getName());
}
}
}

Expand Down Expand Up @@ -1759,6 +1800,10 @@ namespace {
contextStack.push_back(func);
}

if (auto *PBD = dyn_cast<PatternBindingDecl>(decl)) {
patternBindingStack.push_back(PBD);
}

return true;
}

Expand All @@ -1768,6 +1813,11 @@ namespace {
contextStack.pop_back();
}

if (auto *PBD = dyn_cast<PatternBindingDecl>(decl)) {
assert(patternBindingStack.back() == PBD);
patternBindingStack.pop_back();
}

return true;
}

Expand Down
4 changes: 2 additions & 2 deletions test/Concurrency/concurrent_value_checking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ var concurrentFuncVar: (@Sendable (NotConcurrent) -> Void)? = nil
// ----------------------------------------------------------------------
func acceptConcurrentUnary<T>(_: @Sendable (T) -> T) { }

func concurrentClosures<T>(_: T) {
func concurrentClosures<T>(_: T) { // expected-note{{consider making generic parameter 'T' conform to the 'Sendable' protocol}} {{26-26=: Sendable}}
acceptConcurrentUnary { (x: T) in
_ = x // ok
acceptConcurrentUnary { _ in x } // expected-warning{{capture of 'x' with non-sendable type 'T' in a `@Sendable` closure}}
Expand All @@ -269,7 +269,7 @@ struct S1: Sendable {
var nc: NotConcurrent // expected-warning{{stored property 'nc' of 'Sendable'-conforming struct 'S1' has non-sendable type 'NotConcurrent'}}
}

struct S2<T>: Sendable {
struct S2<T>: Sendable { // expected-note{{consider making generic parameter 'T' conform to the 'Sendable' protocol}} {{12-12=: Sendable}}
var nc: T // expected-warning{{stored property 'nc' of 'Sendable'-conforming generic struct 'S2' has non-sendable type 'T'}}
}

Expand Down
27 changes: 17 additions & 10 deletions test/Concurrency/sr15049.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,33 @@ func testAsyncSequence1Sendable<Seq: AsyncSequence>(_ seq: Seq) async throws whe
async let _ = seq.reduce(0) { $0 + $1 } // OK
}

// TODO(diagnostics) [SR-16092]: Add a tailored wording for implicit autoclosure captures in sendable warnings, because
// from user perspective there is no closure capture, so diagnostic can be confusing.
func testAsyncSequenceTypedPattern<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
func testAsyncSequenceTypedPattern<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-note{{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
async let result: Int = seq.reduce(0) { $0 + $1 } // OK
// expected-warning@-1{{immutable value 'result' was never used; consider replacing with '_' or removing it}}
// expected-warning@-2{{capture of 'seq' with non-sendable type 'Seq' in a `@Sendable` closure}}
// expected-warning@-2{{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func testAsyncSequenceTypedPattern1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
func testAsyncSequenceTypedPattern1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-note{{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
async let _: Int = seq.reduce(0) { $0 + $1 } // OK
// expected-warning@-1{{capture of 'seq' with non-sendable type 'Seq' in a `@Sendable` closure}}
// expected-warning@-1{{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func testAsyncSequence<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
func testAsyncSequence<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-note{{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
async let result = seq.reduce(0) { $0 + $1 } // OK
// expected-warning@-1{{initialization of immutable value 'result' was never used; consider replacing with assignment to '_' or removing it}}
// expected-warning@-2{{capture of 'seq' with non-sendable type 'Seq' in a `@Sendable` closure}}
// expected-warning@-2{{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func testAsyncSequence1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
func testAsyncSequence1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-note{{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
async let _ = seq.reduce(0) { $0 + $1 } // OK
// expected-warning@-1{{capture of 'seq' with non-sendable type 'Seq' in a `@Sendable` closure}}
// expected-warning@-1{{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func testAsyncSequence3<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-note{{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
async let result = seq // expected-warning{{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
//expected-warning@-1{{initialization of immutable value 'result' was never used; consider replacing with assignment to '_' or removing it}}
}

func testAsyncSequence4<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-note{{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
async let _ = seq // expected-warning{{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}
1 change: 1 addition & 0 deletions test/Distributed/distributed_protocol_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ protocol Server {
func send<Message: Codable>(message: Message) async throws -> String
}
actor MyServer : Server {
// expected-note@+1{{consider making generic parameter 'Message' conform to the 'Sendable' protocol}} {{29-29=, Sendable}}
func send<Message: Codable>(message: Message) throws -> String { "" } // expected-warning{{non-sendable type 'Message' in parameter of actor-isolated instance method 'send(message:)' satisfying non-isolated protocol requirement cannot cross actor boundary}}
}

Expand Down