Skip to content

Commit 94a2529

Browse files
committed
Teach diagnostic about non-Sendable global lets to respect @preconcurrency
The diagnostic for non-Sendable globa/static `let` properties was checking for a Sendable conformance without considering `@preconcurrency`. Emit this diagnostic via a `@preconcurrency`-sensitive path. Fixes rdar://121889248.
1 parent 1257db7 commit 94a2529

10 files changed

+86
-26
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5614,9 +5614,9 @@ NOTE(shared_mutable_state_decl_note,none,
56145614
"isolate %0 to a global actor, or convert it to a 'let' constant and "
56155615
"conform it to 'Sendable'", (const ValueDecl *))
56165616
ERROR(shared_immutable_state_decl,none,
5617-
"%kind0 is not concurrency-safe because non-'Sendable' type %1 may have "
5617+
"%kind1 is not concurrency-safe because non-'Sendable' type %0 may have "
56185618
"shared mutable state",
5619-
(const ValueDecl *, Type))
5619+
(Type, const ValueDecl *))
56205620
NOTE(shared_immutable_state_decl_note,none,
56215621
"isolate %0 to a global actor, or conform %1 to 'Sendable'",
56225622
(const ValueDecl *, Type))

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5013,13 +5013,17 @@ ActorIsolation ActorIsolationRequest::evaluate(
50135013
}
50145014
if (var->isLet()) {
50155015
auto type = var->getInterfaceType();
5016-
if (!type->isSendableType()) {
5017-
diagVar->diagnose(diag::shared_immutable_state_decl,
5018-
diagVar, type)
5019-
.warnUntilSwiftVersion(6);
5016+
bool diagnosed = diagnoseIfAnyNonSendableTypes(
5017+
type, SendableCheckContext(var->getDeclContext()),
5018+
/*inDerivedConformance=*/Type(), /*typeLoc=*/SourceLoc(),
5019+
/*diagnoseLoc=*/var->getLoc(),
5020+
diag::shared_immutable_state_decl, diagVar);
5021+
5022+
// If we diagnosed this 'let' as non-Sendable, tack on a note
5023+
// to suggest a course of action.
5024+
if (diagnosed)
50205025
diagVar->diagnose(diag::shared_immutable_state_decl_note,
50215026
diagVar, type);
5022-
}
50235027
} else {
50245028
diagVar->diagnose(diag::shared_mutable_state_decl, diagVar)
50255029
.warnUntilSwiftVersion(6);

lib/Sema/TypeCheckConcurrency.h

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -436,20 +436,64 @@ bool diagnoseNonSendableTypes(
436436
Diag<Type, DiagArgs...> diag,
437437
typename detail::Identity<DiagArgs>::type ...diagArgs) {
438438

439-
ASTContext &ctx = fromContext.fromDC->getASTContext();
440-
return diagnoseNonSendableTypes(
441-
type, fromContext, derivedConformance, typeLoc,
442-
[&](Type specificType, DiagnosticBehavior behavior) {
443-
auto preconcurrency =
444-
fromContext.preconcurrencyBehavior(type->getAnyNominal());
439+
ASTContext &ctx = fromContext.fromDC->getASTContext();
440+
return diagnoseNonSendableTypes(
441+
type, fromContext, derivedConformance, typeLoc,
442+
[&](Type specificType, DiagnosticBehavior behavior) {
443+
auto preconcurrency =
444+
fromContext.preconcurrencyBehavior(type->getAnyNominal());
445+
446+
ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
447+
.limitBehaviorUntilSwiftVersion(behavior, 6)
448+
.limitBehaviorIf(preconcurrency);
449+
450+
return (behavior == DiagnosticBehavior::Ignore ||
451+
preconcurrency == DiagnosticBehavior::Ignore);
452+
});
453+
}
454+
455+
/// Emit a diagnostic if there are any non-Sendable types for which
456+
/// the Sendable diagnostic wasn't suppressed. This diagnostic will
457+
/// only be emitted once, but there might be additional notes for the
458+
/// various occurrences of Sendable types.
459+
///
460+
/// \param typeLoc is the source location of the type being diagnosed
461+
///
462+
/// \param diagnoseLoc is the source location at which the main diagnostic should
463+
/// be reported, which can differ from typeLoc
464+
///
465+
/// \returns \c true if any diagnostics.
466+
template<typename ...DiagArgs>
467+
bool diagnoseIfAnyNonSendableTypes(
468+
Type type, SendableCheckContext fromContext,
469+
Type derivedConformance,
470+
SourceLoc typeLoc, SourceLoc diagnoseLoc,
471+
Diag<Type, DiagArgs...> diag,
472+
typename detail::Identity<DiagArgs>::type ...diagArgs) {
473+
474+
ASTContext &ctx = fromContext.fromDC->getASTContext();
475+
bool diagnosed = false;
476+
diagnoseNonSendableTypes(
477+
type, fromContext, derivedConformance, typeLoc,
478+
[&](Type specificType, DiagnosticBehavior behavior) {
479+
auto preconcurrency =
480+
fromContext.preconcurrencyBehavior(type->getAnyNominal());
445481

482+
if (behavior == DiagnosticBehavior::Ignore ||
483+
preconcurrency == DiagnosticBehavior::Ignore)
484+
return true;
485+
486+
if (!diagnosed) {
446487
ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
447488
.limitBehaviorUntilSwiftVersion(behavior, 6)
448489
.limitBehaviorIf(preconcurrency);
490+
diagnosed = true;
491+
}
492+
493+
return false;
494+
});
449495

450-
return (behavior == DiagnosticBehavior::Ignore ||
451-
preconcurrency == DiagnosticBehavior::Ignore);
452-
});
496+
return diagnosed;
453497
}
454498

455499
/// Diagnose any non-Sendable types that occur within the given type, using

test/Concurrency/Inputs/NonStrictModule.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
public struct NonStrictStruct { }
22

33
open class NonStrictClass {
4+
public init() {}
45
open func send(_ body: @Sendable () -> Void) {}
56
open func dontSend(_ body: () -> Void) {}
67
}

test/Concurrency/Inputs/StrictModule.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
public struct StrictStruct: Hashable { }
1+
public struct StrictStruct: Hashable {
2+
public init() {}
3+
}
24

35
open class StrictClass {
46
open func send(_ body: @Sendable () -> Void) {}

test/Concurrency/concurrency_warnings.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// REQUIRES: concurrency
55
// REQUIRES: asserts
66

7-
class GlobalCounter {
7+
class GlobalCounter { // expected-note{{class 'GlobalCounter' does not conform to the 'Sendable' protocol}}
88
var counter: Int = 0
99
}
1010

test/Concurrency/global_variables.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ final class TestSendable: Sendable {
2323
init() {}
2424
}
2525

26-
final class TestNonsendable {
26+
final class TestNonsendable { // expected-note 2{{class 'TestNonsendable' does not conform to the 'Sendable' protocol}}
2727
init() {}
2828
}
2929

test/Concurrency/predates_concurrency_import.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
44
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
55

6-
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify
7-
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted -disable-region-based-isolation-with-strict-concurrency
8-
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency
9-
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
6+
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -parse-as-library -enable-upcoming-feature GlobalConcurrency
7+
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted -disable-region-based-isolation-with-strict-concurrency -parse-as-library -enable-upcoming-feature GlobalConcurrency
8+
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -disable-region-based-isolation-with-strict-concurrency -parse-as-library -enable-upcoming-feature GlobalConcurrency
9+
// RUN: %target-swift-frontend -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -parse-as-library -enable-upcoming-feature GlobalConcurrency
1010

1111
// REQUIRES: concurrency
1212
// REQUIRES: asserts
@@ -28,3 +28,8 @@ func test(
2828
acceptSendable(oma) // okay
2929
acceptSendable(ssc) // okay
3030
}
31+
32+
let nonStrictGlobal = NonStrictClass() // no warning
33+
34+
let strictGlobal = StrictStruct() // expected-warning{{let 'strictGlobal' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
35+
// expected-note@-1{{isolate 'strictGlobal' to a global actor, or conform 'StrictStruct' to 'Sendable'}}

test/Concurrency/predates_concurrency_import_swift6.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -swift-version 6 %S/Inputs/StrictModule.swift
33
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
44

5-
// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify
6-
// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation
5+
// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -parse-as-library
6+
// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation -parse-as-library
77

88
@preconcurrency import NonStrictModule
99
@preconcurrency import StrictModule
@@ -15,3 +15,7 @@ func test(ss: StrictStruct, ns: NonStrictClass) {
1515
acceptSendable(ss) // expected-warning{{type 'StrictStruct' does not conform to the 'Sendable' protocol}}
1616
acceptSendable(ns)
1717
}
18+
19+
let nonStrictGlobal = NonStrictClass()
20+
let strictGlobal = StrictStruct() // expected-warning{{let 'strictGlobal' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
21+
// expected-note@-1{{isolate 'strictGlobal' to a global actor, or conform 'StrictStruct' to 'Sendable'}}

test/Concurrency/sendable_cycle.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// REQUIRES: concurrency
77
// REQUIRES: asserts
88

9-
struct Bar {
9+
struct Bar { // expected-note*{{consider making struct 'Bar' conform to the 'Sendable' protocol}}
1010
lazy var foo = { // expected-error {{escaping closure captures mutating 'self' parameter}}
1111
self.x() // expected-note {{captured here}}
1212
}

0 commit comments

Comments
 (0)