From 3211bd8260bc4efd4e6e73a6569d9994a3f49d76 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Tue, 14 Jun 2022 17:39:18 -0700 Subject: [PATCH] Emit Sendable diagnostics in actor-like deinits only in 'complete' checking mode The flow-isolation pass was not respecting the new strict-concurrency checking mode. Since the Sendable diagnostics in these deinits are very noisy, I'm moving them to only be emitted in 'complete' mode. The reason why they're so noisy is that any class that inherits from a `@MainActor`-constrained class will have these diagnostics emitted when trying to access its own `@MainActor`-isolated members. This is needed, even during the `deinit`, because multiple instances of a `@MainActor`-isolated class might have stored properties that refer to the same state. This change specifically avoids emitting these diagnostics even in 'targeted' mode because I'd like to take more time to reconsider the ergonomics of these deinits. resolves rdar://94699928 --- lib/SILOptimizer/Mandatory/FlowIsolation.cpp | 12 +++++++ test/Concurrency/flow_isolation.swift | 32 +++++++++++++++++-- .../flow_isolation_nonstrict.swift | 30 +++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 test/Concurrency/flow_isolation_nonstrict.swift diff --git a/lib/SILOptimizer/Mandatory/FlowIsolation.cpp b/lib/SILOptimizer/Mandatory/FlowIsolation.cpp index 21b7d318be6c7..c2dbae8420def 100644 --- a/lib/SILOptimizer/Mandatory/FlowIsolation.cpp +++ b/lib/SILOptimizer/Mandatory/FlowIsolation.cpp @@ -511,6 +511,18 @@ static bool diagnoseNonSendableFromDeinit(ModuleDecl *module, RefElementAddrInst *inst) { VarDecl *var = inst->getField(); Type ty = var->getType(); + DeclContext* dc = inst->getFunction()->getDeclContext(); + +// FIXME: we should emit diagnostics in other modes using: +// +// if (!shouldDiagnoseExistingDataRaces(dc)) +// return false; +// +// but until we decide how we want to handle isolated state from deinits, +// we're going to limit the noise to complete mode for now. + if (dc->getASTContext().LangOpts.StrictConcurrencyLevel + != StrictConcurrency::Complete) + return false; if (isSendableType(module, ty)) return false; diff --git a/test/Concurrency/flow_isolation.swift b/test/Concurrency/flow_isolation.swift index f1da881c454ec..42736dee9f868 100644 --- a/test/Concurrency/flow_isolation.swift +++ b/test/Concurrency/flow_isolation.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -swift-version 5 -parse-as-library -emit-sil -verify %s +// RUN: %target-swift-frontend -strict-concurrency=complete -swift-version 5 -parse-as-library -emit-sil -verify %s func randomBool() -> Bool { return false } func logTransaction(_ i: Int) {} @@ -16,6 +16,7 @@ func takeSendable(_ s: SendableType) {} class NonSendableType { var x: Int = 0 + func f() {} } @available(SwiftStdlib 5.1, *) @@ -514,7 +515,7 @@ struct CardboardBox { @available(SwiftStdlib 5.1, *) -var globalVar: EscapeArtist? +var globalVar: EscapeArtist? // expected-note 2 {{var declared here}} @available(SwiftStdlib 5.1, *) actor EscapeArtist { @@ -523,7 +524,11 @@ actor EscapeArtist { init(attempt1: Bool) { self.x = 0 - globalVar = self // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}} + // 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}} + 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 Swift 6}} @@ -700,3 +705,24 @@ actor OhBrother { whatever = 2 // expected-warning {{cannot access property 'whatever' here in non-isolated initializer; this is an error in Swift 6}} } } + +@available(SwiftStdlib 5.1, *) +@MainActor class AwesomeUIView {} + +@available(SwiftStdlib 5.1, *) +class CheckDeinitFromClass: AwesomeUIView { + var ns: NonSendableType? + deinit { + ns?.f() // expected-warning {{cannot access property 'ns' with a non-sendable type 'NonSendableType?' from non-isolated deinit; this is an error in Swift 6}} + ns = nil // expected-warning {{cannot access property 'ns' with a non-sendable type 'NonSendableType?' from non-isolated deinit; this is an error in Swift 6}} + } +} + +@available(SwiftStdlib 5.1, *) +actor CheckDeinitFromActor { + var ns: NonSendableType? + deinit { + ns?.f() // expected-warning {{cannot access property 'ns' with a non-sendable type 'NonSendableType?' from non-isolated deinit; this is an error in Swift 6}} + ns = nil // expected-warning {{cannot access property 'ns' with a non-sendable type 'NonSendableType?' from non-isolated deinit; this is an error in Swift 6}} + } +} diff --git a/test/Concurrency/flow_isolation_nonstrict.swift b/test/Concurrency/flow_isolation_nonstrict.swift new file mode 100644 index 0000000000000..8a94636eb5cf4 --- /dev/null +++ b/test/Concurrency/flow_isolation_nonstrict.swift @@ -0,0 +1,30 @@ +// RUN: %target-swift-frontend -strict-concurrency=targeted -swift-version 5 -parse-as-library -emit-sil -verify %s + +class NonSendableType { + var x: Int = 0 + func f() {} +} + +// rdar://94699928 - don't emit sendable diagnostics in non-'complete' mode +// for deinits of actor or global-actor isolated types + +@available(SwiftStdlib 5.1, *) +@MainActor class AwesomeUIView {} + +@available(SwiftStdlib 5.1, *) +class CheckDeinitFromClass: AwesomeUIView { + var ns: NonSendableType? + deinit { + ns?.f() + ns = nil + } +} + +@available(SwiftStdlib 5.1, *) +actor CheckDeinitFromActor { + var ns: NonSendableType? + deinit { + ns?.f() + ns = nil + } +}