Skip to content

Commit b1a8ad8

Browse files
Merge pull request #70421 from sophiapoirier/globals-preconcurrency-warning
@preconcurrency import downgrades globals concurrency errors to warnings
2 parents f7f5070 + 223a9ee commit b1a8ad8

File tree

6 files changed

+139
-84
lines changed

6 files changed

+139
-84
lines changed

include/swift/AST/DiagnosticEngine.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,16 @@ namespace swift {
564564
/// emitted as a warning, but a note will still be emitted as a note.
565565
InFlightDiagnostic &limitBehavior(DiagnosticBehavior limit);
566566

567+
/// Conditionally prevent the diagnostic from behaving more severely than \p
568+
/// limit. If the condition is false, no limit is imposed.
569+
InFlightDiagnostic &limitBehaviorIf(bool shouldLimit,
570+
DiagnosticBehavior limit) {
571+
if (!shouldLimit) {
572+
return *this;
573+
}
574+
return limitBehavior(limit);
575+
}
576+
567577
/// Limit the diagnostic behavior to warning until the specified version.
568578
///
569579
/// This helps stage in fixes for stricter diagnostics as warnings

include/swift/AST/DiagnosticsSema.def

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5518,18 +5518,18 @@ ERROR(distributed_actor_remote_func_must_not_be_distributed,none,
55185518
NOTE(actor_mutable_state,none,
55195519
"mutation of this %0 is only permitted within the actor",
55205520
(DescriptiveDeclKind))
5521-
WARNING(shared_mutable_state_access,none,
5522-
"reference to %kind0 is not concurrency-safe because it involves "
5523-
"shared mutable state", (const ValueDecl *))
5524-
WARNING(shared_mutable_state_decl,none,
5525-
"%kind0 is not concurrency-safe because it is non-isolated global "
5526-
"shared mutable state", (const ValueDecl *))
5521+
ERROR(shared_mutable_state_access,none,
5522+
"reference to %kind0 is not concurrency-safe because it involves shared "
5523+
"mutable state", (const ValueDecl *))
5524+
ERROR(shared_mutable_state_decl,none,
5525+
"%kind0 is not concurrency-safe because it is non-isolated global shared "
5526+
"mutable state", (const ValueDecl *))
55275527
NOTE(shared_mutable_state_decl_note,none,
55285528
"isolate %0 to a global actor, or convert it to a 'let' constant and "
55295529
"conform it to 'Sendable'", (const ValueDecl *))
5530-
WARNING(shared_immutable_state_decl,none,
5531-
"%kind0 is not concurrency-safe because it is not either conforming to "
5532-
"'Sendable' or isolated to a global actor", (const ValueDecl *))
5530+
ERROR(shared_immutable_state_decl,none,
5531+
"%kind0 is not concurrency-safe because it is not either conforming to "
5532+
"'Sendable' or isolated to a global actor", (const ValueDecl *))
55335533
ERROR(actor_isolated_witness,none,
55345534
"%select{|distributed }0%1 %kind2 cannot be used to satisfy %3 protocol "
55355535
"requirement",

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -829,12 +829,12 @@ static bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
829829
conformance.getConcrete())->isMissing());
830830
}
831831

832-
/// Find the import that makes the given nominal declaration available.
832+
/// Find the import that makes the given declaration available.
833833
static llvm::Optional<AttributedImport<ImportedModule>>
834-
findImportFor(NominalTypeDecl *nominal, const DeclContext *fromDC) {
835-
// If the nominal type is from the current module, there's no import.
836-
auto nominalModule = nominal->getParentModule();
837-
if (nominalModule == fromDC->getParentModule())
834+
findImportFor(const DeclContext *dc, const DeclContext *fromDC) {
835+
// If the type is from the current module, there's no import.
836+
auto module = dc->getParentModule();
837+
if (module == fromDC->getParentModule())
838838
return llvm::None;
839839

840840
auto fromSourceFile = fromDC->getParentSourceFile();
@@ -843,16 +843,16 @@ findImportFor(NominalTypeDecl *nominal, const DeclContext *fromDC) {
843843

844844
// Look to see if the owning module was directly imported.
845845
for (const auto &import : fromSourceFile->getImports()) {
846-
if (import.module.importedModule == nominalModule)
846+
if (import.module.importedModule == module)
847847
return import;
848848
}
849849

850850
// Now look for transitive imports.
851-
auto &importCache = nominal->getASTContext().getImportCache();
851+
auto &importCache = dc->getASTContext().getImportCache();
852852
for (const auto &import : fromSourceFile->getImports()) {
853853
auto &importSet = importCache.getImportSet(import.module.importedModule);
854854
for (const auto &transitive : importSet.getTransitiveImports()) {
855-
if (transitive.importedModule == nominalModule) {
855+
if (transitive.importedModule == module) {
856856
return import;
857857
}
858858
}
@@ -2931,7 +2931,8 @@ namespace {
29312931
///
29322932
/// \returns true if we diagnosed the entity, \c false otherwise.
29332933
bool diagnoseReferenceToUnsafeGlobal(ValueDecl *value, SourceLoc loc) {
2934-
switch (value->getASTContext().LangOpts.StrictConcurrencyLevel) {
2934+
auto &ctx = value->getASTContext();
2935+
switch (ctx.LangOpts.StrictConcurrencyLevel) {
29352936
case StrictConcurrency::Minimal:
29362937
case StrictConcurrency::Targeted:
29372938
// Never diagnose.
@@ -2954,16 +2955,32 @@ namespace {
29542955
return false;
29552956

29562957
// If it's actor-isolated, it's already been dealt with.
2957-
if (getActorIsolation(value).isActorIsolated())
2958+
const auto isolation = getActorIsolation(value);
2959+
if (isolation.isActorIsolated())
29582960
return false;
29592961

29602962
if (auto attr = value->getAttrs().getAttribute<NonisolatedAttr>();
29612963
attr && attr->isUnsafe()) {
29622964
return false;
29632965
}
29642966

2965-
ctx.Diags.diagnose(loc, diag::shared_mutable_state_access, value);
2967+
const auto import =
2968+
findImportFor(var->getDeclContext(), getDeclContext());
2969+
const bool isPreconcurrencyImport =
2970+
import && import->options.contains(ImportFlags::Preconcurrency);
2971+
const auto isPreconcurrencyUnspecifiedIsolation =
2972+
isPreconcurrencyImport && isolation.isUnspecified();
2973+
ctx.Diags.diagnose(loc, diag::shared_mutable_state_access, value)
2974+
.warnUntilSwiftVersionIf(!isPreconcurrencyUnspecifiedIsolation, 6)
2975+
.limitBehaviorIf(isPreconcurrencyImport, DiagnosticBehavior::Warning)
2976+
.limitBehaviorIf(!ctx.isSwiftVersionAtLeast(6) &&
2977+
isPreconcurrencyUnspecifiedIsolation,
2978+
DiagnosticBehavior::Ignore);
29662979
value->diagnose(diag::kind_declared_here, value->getDescriptiveKind());
2980+
if (const auto sourceFile = getDeclContext()->getParentSourceFile();
2981+
sourceFile && isPreconcurrencyImport) {
2982+
sourceFile->setImportUsedPreconcurrency(*import);
2983+
}
29672984
return true;
29682985
}
29692986

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
public struct Globals {
2+
public static let integerConstant = 0
3+
public static var integerMutable = 0
4+
5+
public static nonisolated(unsafe) let nonisolatedUnsafeIntegerConstant = 0
6+
public static nonisolated(unsafe) var nonisolatedUnsafeIntegerMutable = 0
7+
8+
@MainActor public static var actorInteger = 0
9+
10+
public init() {}
11+
}

test/Concurrency/experimental_feature_strictconcurrency.swift

Lines changed: 3 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify %s
2-
// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify %s
3-
// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -emit-sil -o /dev/null -verify -verify-additional-prefix region-isolation- -enable-experimental-feature RegionBasedIsolation %s
1+
// RUN: %target-swift-frontend -disable-availability-checking -enable-experimental-feature StrictConcurrency -emit-sil -o /dev/null -verify %s
2+
// RUN: %target-swift-frontend -disable-availability-checking -enable-experimental-feature StrictConcurrency=complete -emit-sil -o /dev/null -verify %s
3+
// RUN: %target-swift-frontend -disable-availability-checking -enable-experimental-feature StrictConcurrency=complete -emit-sil -o /dev/null -verify -verify-additional-prefix region-isolation- -enable-experimental-feature RegionBasedIsolation %s
44

55
// REQUIRES: concurrency
66
// REQUIRES: asserts
@@ -24,67 +24,6 @@ struct Test2: TestProtocol { // expected-warning{{conformance of 'C2' to 'Sendab
2424
typealias Value = C2
2525
}
2626

27-
@globalActor
28-
actor TestGlobalActor {
29-
static var shared = TestGlobalActor()
30-
}
31-
32-
@TestGlobalActor
33-
var mutableIsolatedGlobal = 1
34-
35-
var mutableNonisolatedGlobal = 1 // expected-warning{{var 'mutableNonisolatedGlobal' is not concurrency-safe because it is non-isolated global shared mutable state}}
36-
// expected-note@-1{{isolate 'mutableNonisolatedGlobal' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
37-
38-
let immutableGlobal = 1
39-
40-
final class TestSendable: Sendable {
41-
init() {}
42-
}
43-
44-
final class TestNonsendable {
45-
init() {}
46-
}
47-
48-
nonisolated(unsafe) let immutableNonisolatedUnsafeTopLevelGlobal = TestNonsendable()
49-
50-
@propertyWrapper
51-
public struct TestWrapper {
52-
public init() {}
53-
public var wrappedValue: Int {
54-
return 0
55-
}
56-
}
57-
58-
struct TestStatics {
59-
static let immutableExplicitSendable = TestSendable()
60-
static let immutableNonsendable = TestNonsendable() // expected-warning{{static property 'immutableNonsendable' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor}}
61-
static nonisolated(unsafe) let immutableNonisolatedUnsafe = TestNonsendable()
62-
static nonisolated let immutableNonisolated = TestNonsendable() // expected-warning{{static property 'immutableNonisolated' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor}}
63-
static let immutableInferredSendable = 0
64-
static var mutable = 0 // expected-warning{{static property 'mutable' is not concurrency-safe because it is non-isolated global shared mutable state}}
65-
// expected-note@-1{{isolate 'mutable' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
66-
// expected-note@-2{{static property declared here}}
67-
static var computedProperty: Int { 0 } // computed property that, though static, has no storage so is not a global
68-
@TestWrapper static var wrapped: Int // expected-warning{{static property 'wrapped' is not concurrency-safe because it is non-isolated global shared mutable state}}
69-
// expected-note@-1{{isolate 'wrapped' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
70-
}
71-
72-
@TestGlobalActor
73-
func f() {
74-
print(TestStatics.immutableExplicitSendable)
75-
print(TestStatics.immutableInferredSendable)
76-
print(TestStatics.mutable) // expected-warning{{reference to static property 'mutable' is not concurrency-safe because it involves shared mutable state}}
77-
}
78-
79-
func testLocalNonisolatedUnsafe() async {
80-
nonisolated(unsafe) var value = 1
81-
let task = Task {
82-
value = 2
83-
return value
84-
}
85-
print(await task.value)
86-
}
87-
8827
@MainActor
8928
func iterate(stream: AsyncStream<Int>) async {
9029
nonisolated(unsafe) var it = stream.makeAsyncIterator()
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/GlobalVariables.swiftmodule -module-name GlobalVariables -parse-as-library -strict-concurrency=minimal -swift-version 5 %S/Inputs/GlobalVariables.swift
3+
// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify %s
4+
5+
// REQUIRES: concurrency
6+
// REQUIRES: asserts
7+
8+
@preconcurrency import GlobalVariables
9+
10+
@globalActor
11+
actor TestGlobalActor {
12+
static var shared = TestGlobalActor()
13+
}
14+
15+
@TestGlobalActor
16+
var mutableIsolatedGlobal = 1
17+
18+
var mutableNonisolatedGlobal = 1 // expected-error{{var 'mutableNonisolatedGlobal' is not concurrency-safe because it is non-isolated global shared mutable state}}
19+
// expected-note@-1{{isolate 'mutableNonisolatedGlobal' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
20+
21+
let immutableGlobal = 1
22+
23+
final class TestSendable: Sendable {
24+
init() {}
25+
}
26+
27+
final class TestNonsendable {
28+
init() {}
29+
}
30+
31+
nonisolated(unsafe) let immutableNonisolatedUnsafeTopLevelGlobal = TestNonsendable()
32+
33+
@propertyWrapper
34+
public struct TestWrapper {
35+
public init() {}
36+
public var wrappedValue: Int {
37+
return 0
38+
}
39+
}
40+
41+
struct TestStatics {
42+
static let immutableExplicitSendable = TestSendable()
43+
static let immutableNonsendable = TestNonsendable() // expected-error{{static property 'immutableNonsendable' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor}}
44+
static nonisolated(unsafe) let immutableNonisolatedUnsafe = TestNonsendable()
45+
static nonisolated let immutableNonisolated = TestNonsendable() // expected-error{{static property 'immutableNonisolated' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor}}
46+
static let immutableInferredSendable = 0
47+
static var mutable = 0 // expected-error{{static property 'mutable' is not concurrency-safe because it is non-isolated global shared mutable state}}
48+
// expected-note@-1{{isolate 'mutable' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
49+
// expected-note@-2{{static property declared here}}
50+
static var computedProperty: Int { 0 } // computed property that, though static, has no storage so is not a global
51+
@TestWrapper static var wrapped: Int // expected-error{{static property 'wrapped' is not concurrency-safe because it is non-isolated global shared mutable state}}
52+
// expected-note@-1{{isolate 'wrapped' to a global actor, or convert it to a 'let' constant and conform it to 'Sendable'}}
53+
}
54+
55+
@TestGlobalActor
56+
func f() {
57+
print(TestStatics.immutableExplicitSendable)
58+
print(TestStatics.immutableInferredSendable)
59+
print(TestStatics.mutable) // expected-error{{reference to static property 'mutable' is not concurrency-safe because it involves shared mutable state}}
60+
print(Globals.actorInteger) // expected-error{{main actor-isolated static property 'actorInteger' can not be referenced from global actor 'TestGlobalActor'}}
61+
}
62+
63+
func testLocalNonisolatedUnsafe() async {
64+
nonisolated(unsafe) var value = 1
65+
let task = Task {
66+
value = 2
67+
return value
68+
}
69+
print(await task.value)
70+
}
71+
72+
func testImportedGlobals() { // expected-note{{add '@MainActor' to make global function 'testImportedGlobals()' part of global actor 'MainActor'}}
73+
let _ = Globals.integerConstant
74+
let _ = Globals.integerMutable // expected-warning{{reference to static property 'integerMutable' is not concurrency-safe because it involves shared mutable state}}
75+
let _ = Globals.nonisolatedUnsafeIntegerConstant
76+
let _ = Globals.nonisolatedUnsafeIntegerMutable
77+
let _ = Globals.actorInteger // expected-error{{main actor-isolated static property 'actorInteger' can not be referenced from a non-isolated context}}
78+
}

0 commit comments

Comments
 (0)