Skip to content

Commit 8749d6e

Browse files
committed
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
1 parent 5141a1e commit 8749d6e

File tree

5 files changed

+78
-10
lines changed

5 files changed

+78
-10
lines changed

include/swift/AST/ActorIsolation.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ bool areTypesEqual(Type type1, Type type2);
3838
/// Determine whether the given type is suitable as a concurrent value type.
3939
bool isSendableType(ModuleDecl *module, Type type);
4040

41+
/// Determine whether we should diagnose data races within the current context.
42+
///
43+
/// By default, we do this only in code that makes use of concurrency
44+
/// features.
45+
bool shouldDiagnoseExistingDataRaces(const DeclContext *dc);
46+
4147
/// Determines if the 'let' can be read from anywhere within the given module,
4248
/// regardless of the isolation or async-ness of the context in which
4349
/// the var is read.

lib/SILOptimizer/Mandatory/FlowIsolation.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,18 @@ static bool diagnoseNonSendableFromDeinit(ModuleDecl *module,
511511
RefElementAddrInst *inst) {
512512
VarDecl *var = inst->getField();
513513
Type ty = var->getType();
514+
DeclContext* dc = inst->getFunction()->getDeclContext();
515+
516+
// FIXME: we should emit diagnostics in other modes using:
517+
//
518+
// if (!shouldDiagnoseExistingDataRaces(dc))
519+
// return false;
520+
//
521+
// but until we decide how we want to handle isolated state from deinits,
522+
// we're going to limit the noise to complete mode for now.
523+
if (dc->getASTContext().LangOpts.StrictConcurrencyLevel
524+
!= StrictConcurrency::Complete)
525+
return false;
514526

515527
if (isSendableType(module, ty))
516528
return false;

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -519,12 +519,6 @@ static bool isAsyncCall(const ApplyExpr *call) {
519519
return funcType->isAsync();
520520
}
521521

522-
/// Determine whether we should diagnose data races within the current context.
523-
///
524-
/// By default, we do this only in code that makes use of concurrency
525-
/// features.
526-
static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc);
527-
528522
/// Determine whether this closure should be treated as Sendable.
529523
///
530524
/// \param forActorIsolation Whether this check is for the purposes of
@@ -593,7 +587,7 @@ static void addSendableFixIt(const GenericTypeParamDecl *genericArgument,
593587
}
594588
}
595589

596-
static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
590+
bool swift::shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
597591
return contextRequiresStrictConcurrencyChecking(dc, [](const AbstractClosureExpr *) {
598592
return Type();
599593
});

test/Concurrency/flow_isolation.swift

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -swift-version 5 -parse-as-library -emit-sil -verify %s
1+
// RUN: %target-swift-frontend -strict-concurrency=complete -swift-version 5 -parse-as-library -emit-sil -verify %s
22

33
func randomBool() -> Bool { return false }
44
func logTransaction(_ i: Int) {}
@@ -16,6 +16,7 @@ func takeSendable(_ s: SendableType) {}
1616

1717
class NonSendableType {
1818
var x: Int = 0
19+
func f() {}
1920
}
2021

2122
@available(SwiftStdlib 5.1, *)
@@ -514,7 +515,7 @@ struct CardboardBox<T> {
514515

515516

516517
@available(SwiftStdlib 5.1, *)
517-
var globalVar: EscapeArtist?
518+
var globalVar: EscapeArtist? // expected-note 2 {{var declared here}}
518519

519520
@available(SwiftStdlib 5.1, *)
520521
actor EscapeArtist {
@@ -523,7 +524,11 @@ actor EscapeArtist {
523524
init(attempt1: Bool) {
524525
self.x = 0
525526

526-
globalVar = self // expected-note {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}}
527+
// expected-note@+2 {{after making a copy of 'self', only non-isolated properties of 'self' can be accessed from this init}}
528+
// expected-warning@+1 {{reference to var 'globalVar' is not concurrency-safe because it involves shared mutable state}}
529+
globalVar = self
530+
531+
// expected-warning@+1 {{reference to var 'globalVar' is not concurrency-safe because it involves shared mutable state}}
527532
Task { await globalVar!.isolatedMethod() }
528533

529534
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 {
700705
whatever = 2 // expected-warning {{cannot access property 'whatever' here in non-isolated initializer; this is an error in Swift 6}}
701706
}
702707
}
708+
709+
@available(SwiftStdlib 5.1, *)
710+
@MainActor class AwesomeUIView {}
711+
712+
@available(SwiftStdlib 5.1, *)
713+
class CheckDeinitFromClass: AwesomeUIView {
714+
var ns: NonSendableType?
715+
deinit {
716+
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}}
717+
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}}
718+
}
719+
}
720+
721+
@available(SwiftStdlib 5.1, *)
722+
actor CheckDeinitFromActor {
723+
var ns: NonSendableType?
724+
deinit {
725+
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}}
726+
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}}
727+
}
728+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %target-swift-frontend -strict-concurrency=targeted -swift-version 5 -parse-as-library -emit-sil -verify %s
2+
3+
class NonSendableType {
4+
var x: Int = 0
5+
func f() {}
6+
}
7+
8+
// rdar://94699928 - don't emit sendable diagnostics in non-'complete' mode
9+
// for deinits of actor or global-actor isolated types
10+
11+
@available(SwiftStdlib 5.1, *)
12+
@MainActor class AwesomeUIView {}
13+
14+
@available(SwiftStdlib 5.1, *)
15+
class CheckDeinitFromClass: AwesomeUIView {
16+
var ns: NonSendableType?
17+
deinit {
18+
ns?.f()
19+
ns = nil
20+
}
21+
}
22+
23+
@available(SwiftStdlib 5.1, *)
24+
actor CheckDeinitFromActor {
25+
var ns: NonSendableType?
26+
deinit {
27+
ns?.f()
28+
ns = nil
29+
}
30+
}

0 commit comments

Comments
 (0)