Skip to content

Don't diagnose accesses to global/static variables within the same module #73740

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
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
8 changes: 8 additions & 0 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3094,6 +3094,14 @@ namespace {
return false;
}

// If global variable checking is enabled and the global variable is
// from the same module as the reference, we'll already have diagnosed
// the global variable itself.
if (ctx.LangOpts.hasFeature(Feature::GlobalConcurrency) &&
var->getDeclContext()->getParentModule() ==
getDeclContext()->getParentModule())
return false;

const auto import = var->findImport(getDeclContext());
const bool isPreconcurrencyImport =
import && import->options.contains(ImportFlags::Preconcurrency);
Expand Down
3 changes: 3 additions & 0 deletions test/Concurrency/Inputs/GlobalVariables.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ public struct Globals {

public init() {}
}

public var mutableGlobal: String = "can't touch this"
public var globalInt = 17
8 changes: 2 additions & 6 deletions test/Concurrency/actor_isolation.swift
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
// RUN: %empty-directory(%t)

// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
// 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

// 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
// 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

// REQUIRES: concurrency
// REQUIRES: asserts

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

let immutableGlobal: String = "hello"

// 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}}
// expected-note@+3 {{convert 'mutableGlobal' to a 'let' constant to make the shared state immutable}}
// expected-note@+2 {{restrict 'mutableGlobal' to the main actor if it will only be accessed from the main thread}}
// expected-note@+1 {{unsafely mark 'mutableGlobal' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
var mutableGlobal: String = "can't touch this" // expected-note 5{{var declared here}}

@available(SwiftStdlib 5.1, *)
func globalFunc() { }
@available(SwiftStdlib 5.1, *)
Expand Down
15 changes: 7 additions & 8 deletions test/Concurrency/concurrency_warnings.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -disable-region-based-isolation-with-strict-concurrency
// RUN: %target-swift-frontend -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
// RUN: %empty-directory(%t)

// 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

// 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
// RUN: %target-swift-frontend -I %t -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify

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


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}}
// expected-note@-1 {{restrict 'globalInt' to the main actor if it will only be accessed from the main thread}}
// expected-note@-2 2{{var declared here}}
// expected-note@-3 {{unsafely mark 'globalInt' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
// expected-note@-4 {{convert 'globalInt' to a 'let' constant to make the shared state immutable}}
import GlobalVariables

class MyError: Error { // expected-warning{{non-final class 'MyError' cannot conform to 'Sendable'; use '@unchecked Sendable'}}
var storage = 0 // expected-warning{{stored property 'storage' of 'Sendable'-conforming class 'MyError' is mutable}}
Expand Down
9 changes: 3 additions & 6 deletions test/Concurrency/flow_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -521,9 +521,8 @@ struct CardboardBox<T> {
@available(SwiftStdlib 5.1, *)
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}}
// expected-note@-1 {{restrict 'globalVar' to the main actor if it will only be accessed from the main thread}}
// expected-note@-2 2 {{var declared here}}
// expected-note@-3 {{unsafely mark 'globalVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
// expected-note@-4 {{convert 'globalVar' to a 'let' constant to make the shared state immutable}}
// expected-note@-2 {{unsafely mark 'globalVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
// expected-note@-3 {{convert 'globalVar' to a 'let' constant to make the shared state immutable}}

@available(SwiftStdlib 5.1, *)
actor EscapeArtist {
Expand All @@ -532,11 +531,9 @@ actor EscapeArtist {
init(attempt1: Bool) {
self.x = 0

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

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

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}}
Expand Down
7 changes: 3 additions & 4 deletions test/Concurrency/freestanding_top_level.swift
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// RUN: %target-swift-frontend -concurrency-model=task-to-thread -typecheck -verify %s
// RUN: %target-swift-frontend -concurrency-model=task-to-thread -typecheck -verify -verify-additional-prefix complete- -strict-concurrency=complete %s

// 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}}
// expected-complete-note@+4 {{restrict 'global' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
// expected-complete-note@+3 {{var declared here}}
// 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}}
// expected-complete-note@+3 {{restrict 'global' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
// expected-complete-note@+2 {{unsafely mark 'global' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
// expected-complete-note@+1 {{convert 'global' to a 'let' constant to make the shared state immutable}}{{1-4=let}}
var global = 10

// 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}}
// No warning because we're in the same module.
print(global)
7 changes: 3 additions & 4 deletions test/Concurrency/global_variables.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ struct TestStatics {
static let immutableInferredSendable = 0
static var mutable = 0 // expected-error{{static property 'mutable' is not concurrency-safe because it is non-isolated global shared mutable state}}
// expected-note@-1{{convert 'mutable' to a 'let' constant to make the shared state immutable}}
// expected-note@-2{{static property declared here}}
// expected-note@-3{{unsafely mark 'mutable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
// expected-note@-4{{restrict 'mutable' to the main actor if it will only be accessed from the main thread}}
// expected-note@-2{{unsafely mark 'mutable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
// expected-note@-3{{restrict 'mutable' to the main actor if it will only be accessed from the main thread}}
static var computedProperty: Int { 0 } // computed property that, though static, has no storage so is not a global
@TestWrapper static var wrapped: Int // expected-error{{static property 'wrapped' is not concurrency-safe because it is non-isolated global shared mutable state}}
// expected-note@-1{{convert 'wrapped' to a 'let' constant to make the shared state immutable}}{{23-26=let}}
Expand All @@ -79,7 +78,7 @@ public actor TestPublicActor {
func f() {
print(TestStatics.immutableExplicitSendable)
print(TestStatics.immutableInferredSendable)
print(TestStatics.mutable) // expected-error{{reference to static property 'mutable' is not concurrency-safe because it involves shared mutable state}}
print(TestStatics.mutable)
print(Globals.actorInteger) // expected-error{{main actor-isolated static property 'actorInteger' can not be referenced from global actor 'TestGlobalActor'}}
}

Expand Down