Skip to content

Commit e1a4a1e

Browse files
committed
Don't diagnose accesses to global/static variables within the same module
When SE-0412 (strict concurrency for global variables) is enabled, each global or static mutable variable will be diagnosed if it isn't explicitly on a global actor or `nonisolated(unsafe)`. Suppress diagnostics for references to such global variables when they are in the same module as the declaration of the global variable itself. While these diagnostics are technically correct, they are not strictly necessary since we've already diagnosed the global variable itself (with more actionable advice), and they tend to pile on the developer in a manner that is not helpful.
1 parent 6eb42df commit e1a4a1e

File tree

7 files changed

+29
-28
lines changed

7 files changed

+29
-28
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3094,6 +3094,14 @@ namespace {
30943094
return false;
30953095
}
30963096

3097+
// If global variable checking is enabled and the global variable is
3098+
// from the same module as the reference, we'll already have diagnosed
3099+
// the global variable itself.
3100+
if (ctx.LangOpts.hasFeature(Feature::GlobalConcurrency) &&
3101+
var->getDeclContext()->getParentModule() ==
3102+
getDeclContext()->getParentModule())
3103+
return false;
3104+
30973105
const auto import = var->findImport(getDeclContext());
30983106
const bool isPreconcurrencyImport =
30993107
import && import->options.contains(ImportFlags::Preconcurrency);

test/Concurrency/Inputs/GlobalVariables.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,6 @@ public struct Globals {
99

1010
public init() {}
1111
}
12+
13+
public var mutableGlobal: String = "can't touch this"
14+
public var globalInt = 17

test/Concurrency/actor_isolation.swift

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11
// RUN: %empty-directory(%t)
22

33
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
4+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/GlobalVariables.swiftmodule -module-name GlobalVariables %S/Inputs/GlobalVariables.swift -disable-availability-checking -parse-as-library
45

56
// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library -emit-sil -o /dev/null -verify -enable-upcoming-feature GlobalActorIsolatedTypesUsability %s
67
// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation -enable-upcoming-feature GlobalActorIsolatedTypesUsability %s
78

89
// REQUIRES: concurrency
910
// REQUIRES: asserts
1011

12+
import GlobalVariables
1113
import OtherActors // expected-warning{{add '@preconcurrency' to suppress 'Sendable'-related warnings from module 'OtherActors'}}{{1-1=@preconcurrency }}
1214

1315
let immutableGlobal: String = "hello"
1416

15-
// expected-warning@+4 {{var 'mutableGlobal' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in the Swift 6 language mode}}
16-
// expected-note@+3 {{convert 'mutableGlobal' to a 'let' constant to make the shared state immutable}}
17-
// expected-note@+2 {{restrict 'mutableGlobal' to the main actor if it will only be accessed from the main thread}}
18-
// expected-note@+1 {{unsafely mark 'mutableGlobal' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
19-
var mutableGlobal: String = "can't touch this" // expected-note 5{{var declared here}}
20-
2117
@available(SwiftStdlib 5.1, *)
2218
func globalFunc() { }
2319
@available(SwiftStdlib 5.1, *)

test/Concurrency/concurrency_warnings.swift

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -disable-region-based-isolation-with-strict-concurrency
2-
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
1+
// RUN: %empty-directory(%t)
2+
3+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/GlobalVariables.swiftmodule -module-name GlobalVariables %S/Inputs/GlobalVariables.swift -disable-availability-checking -parse-as-library
4+
5+
// RUN: %target-swift-frontend -I %t -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -disable-region-based-isolation-with-strict-concurrency
6+
// RUN: %target-swift-frontend -I %t -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
37

48
// REQUIRES: concurrency
59
// REQUIRES: asserts
@@ -12,12 +16,7 @@ let rs = GlobalCounter() // expected-warning {{let 'rs' is not concurrency-safe
1216
// expected-note@-1 {{restrict 'rs' to the main actor if it will only be accessed from the main thread}}
1317
// expected-note@-2 {{unsafely mark 'rs' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
1418

15-
16-
var globalInt = 17 // expected-warning {{var 'globalInt' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in the Swift 6 language mode}}
17-
// expected-note@-1 {{restrict 'globalInt' to the main actor if it will only be accessed from the main thread}}
18-
// expected-note@-2 2{{var declared here}}
19-
// expected-note@-3 {{unsafely mark 'globalInt' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
20-
// expected-note@-4 {{convert 'globalInt' to a 'let' constant to make the shared state immutable}}
19+
import GlobalVariables
2120

2221
class MyError: Error { // expected-warning{{non-final class 'MyError' cannot conform to 'Sendable'; use '@unchecked Sendable'}}
2322
var storage = 0 // expected-warning{{stored property 'storage' of 'Sendable'-conforming class 'MyError' is mutable}}

test/Concurrency/flow_isolation.swift

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,8 @@ struct CardboardBox<T> {
521521
@available(SwiftStdlib 5.1, *)
522522
var globalVar: EscapeArtist? // expected-warning {{var 'globalVar' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in the Swift 6 language mode}}
523523
// expected-note@-1 {{restrict 'globalVar' to the main actor if it will only be accessed from the main thread}}
524-
// expected-note@-2 2 {{var declared here}}
525-
// expected-note@-3 {{unsafely mark 'globalVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
526-
// expected-note@-4 {{convert 'globalVar' to a 'let' constant to make the shared state immutable}}
524+
// expected-note@-2 {{unsafely mark 'globalVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
525+
// expected-note@-3 {{convert 'globalVar' to a 'let' constant to make the shared state immutable}}
527526

528527
@available(SwiftStdlib 5.1, *)
529528
actor EscapeArtist {
@@ -532,11 +531,9 @@ actor EscapeArtist {
532531
init(attempt1: Bool) {
533532
self.x = 0
534533

535-
// expected-note@+2 {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}}
536-
// expected-warning@+1 {{reference to var 'globalVar' is not concurrency-safe because it involves shared mutable state}}
534+
// expected-note@+1 {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}}
537535
globalVar = self
538536

539-
// expected-warning@+1 {{reference to var 'globalVar' is not concurrency-safe because it involves shared mutable state}}
540537
Task { await globalVar!.isolatedMethod() }
541538

542539
if self.x == 0 { // expected-warning {{cannot access property 'x' here in non-isolated initializer; this is an error in the Swift 6 language mode}}
Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
// RUN: %target-swift-frontend -concurrency-model=task-to-thread -typecheck -verify %s
22
// RUN: %target-swift-frontend -concurrency-model=task-to-thread -typecheck -verify -verify-additional-prefix complete- -strict-concurrency=complete %s
33

4-
// expected-complete-warning@+5 {{var 'global' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in the Swift 6 language mode}}
5-
// expected-complete-note@+4 {{restrict 'global' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
6-
// expected-complete-note@+3 {{var declared here}}
4+
// expected-complete-warning@+4 {{var 'global' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in the Swift 6 language mode}}
5+
// expected-complete-note@+3 {{restrict 'global' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
76
// expected-complete-note@+2 {{unsafely mark 'global' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
87
// expected-complete-note@+1 {{convert 'global' to a 'let' constant to make the shared state immutable}}{{1-4=let}}
98
var global = 10
109

11-
// expected-complete-warning@+1 {{reference to var 'global' is not concurrency-safe because it involves shared mutable state; this is an error in the Swift 6 language mode}}
10+
// No warning because we're in the same module.
1211
print(global)

test/Concurrency/global_variables.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,8 @@ struct TestStatics {
6060
static let immutableInferredSendable = 0
6161
static var mutable = 0 // expected-error{{static property 'mutable' is not concurrency-safe because it is non-isolated global shared mutable state}}
6262
// expected-note@-1{{convert 'mutable' to a 'let' constant to make the shared state immutable}}
63-
// expected-note@-2{{static property declared here}}
64-
// expected-note@-3{{unsafely mark 'mutable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
65-
// expected-note@-4{{restrict 'mutable' to the main actor if it will only be accessed from the main thread}}
63+
// expected-note@-2{{unsafely mark 'mutable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
64+
// expected-note@-3{{restrict 'mutable' to the main actor if it will only be accessed from the main thread}}
6665
static var computedProperty: Int { 0 } // computed property that, though static, has no storage so is not a global
6766
@TestWrapper static var wrapped: Int // expected-error{{static property 'wrapped' is not concurrency-safe because it is non-isolated global shared mutable state}}
6867
// expected-note@-1{{convert 'wrapped' to a 'let' constant to make the shared state immutable}}{{23-26=let}}
@@ -79,7 +78,7 @@ public actor TestPublicActor {
7978
func f() {
8079
print(TestStatics.immutableExplicitSendable)
8180
print(TestStatics.immutableInferredSendable)
82-
print(TestStatics.mutable) // expected-error{{reference to static property 'mutable' is not concurrency-safe because it involves shared mutable state}}
81+
print(TestStatics.mutable)
8382
print(Globals.actorInteger) // expected-error{{main actor-isolated static property 'actorInteger' can not be referenced from global actor 'TestGlobalActor'}}
8483
}
8584

0 commit comments

Comments
 (0)