Skip to content

Commit 9f7dff0

Browse files
committed
[SE-0466] Treat explicit "nonisolated" like implicit "nonisolated" on protocols
Make explicit "nonisolated" also not special on protocols, so a nonisolated protocol does not suppress default isolation. SendableMetatype is the proper way to suppress default isolation for a protocol. Unfortunately, these rules made it appear like issue #82168 was fixed, when in fact it was not. Keep the test case, but as a failing test, and we'll investigate separately.
1 parent 436e965 commit 9f7dff0

File tree

4 files changed

+87
-51
lines changed

4 files changed

+87
-51
lines changed

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5282,6 +5282,33 @@ getIsolationFromAttributes(const Decl *decl, bool shouldDiagnose = true,
52825282
llvm_unreachable("Forgot about an attribute?");
52835283
}
52845284

5285+
/// Determine the default isolation for the given declaration context.
5286+
static DefaultIsolation getDefaultIsolationForContext(const DeclContext *dc) {
5287+
// Check whether there is a file-specific setting.
5288+
if (auto *sourceFile = dc->getParentSourceFile()) {
5289+
if (auto defaultIsolationInFile = sourceFile->getDefaultIsolation())
5290+
return defaultIsolationInFile.value();
5291+
}
5292+
5293+
// If we're in the main module, check the language option.
5294+
ASTContext &ctx = dc->getASTContext();
5295+
if (dc->getParentModule() == ctx.MainModule)
5296+
return ctx.LangOpts.DefaultIsolationBehavior;
5297+
5298+
// Otherwise, default to nonisolated.
5299+
return DefaultIsolation::Nonisolated;
5300+
}
5301+
5302+
/// Determines whether explicit 'nonisolated' is different from 'unspecified'
5303+
/// in the given context.
5304+
static bool explicitNonisolatedIsSpecial(const DeclContext *dc) {
5305+
ASTContext &ctx = dc->getASTContext();
5306+
if (ctx.LangOpts.hasFeature(Feature::NoExplicitNonIsolated))
5307+
return false;
5308+
5309+
return getDefaultIsolationForContext(dc) == DefaultIsolation::Nonisolated;
5310+
}
5311+
52855312
/// Infer isolation from witnessed protocol requirements.
52865313
static std::optional<InferredActorIsolation>
52875314
getIsolationFromWitnessedRequirements(ValueDecl *value) {
@@ -5298,11 +5325,13 @@ getIsolationFromWitnessedRequirements(ValueDecl *value) {
52985325
if (dc->getSelfProtocolDecl())
52995326
return std::nullopt;
53005327

5301-
// Prevent isolation inference from requirements if the conforming type
5302-
// has an explicit `nonisolated` attribute.
5303-
if (auto *NTD = dc->getSelfNominalTypeDecl()) {
5304-
if (NTD->getAttrs().hasAttribute<NonisolatedAttr>())
5305-
return std::nullopt;
5328+
if (explicitNonisolatedIsSpecial(dc)) {
5329+
// Prevent isolation inference from requirements if the conforming type
5330+
// has an explicit `nonisolated` attribute.
5331+
if (auto *NTD = dc->getSelfNominalTypeDecl()) {
5332+
if (NTD->getAttrs().hasAttribute<NonisolatedAttr>())
5333+
return std::nullopt;
5334+
}
53065335
}
53075336

53085337
// Walk through each of the conformances in this context, collecting any
@@ -5360,8 +5389,13 @@ getIsolationFromWitnessedRequirements(ValueDecl *value) {
53605389
}
53615390

53625391
case ActorIsolation::GlobalActor:
5392+
break;
5393+
53635394
case ActorIsolation::Nonisolated:
53645395
case ActorIsolation::NonisolatedUnsafe:
5396+
if (!explicitNonisolatedIsSpecial(requirement->getDeclContext()))
5397+
continue;
5398+
53655399
break;
53665400
}
53675401

@@ -5468,7 +5502,8 @@ getIsolationFromConformances(NominalTypeDecl *nominal) {
54685502
case ActorIsolation::NonisolatedUnsafe:
54695503
break;
54705504
case ActorIsolation::Nonisolated:
5471-
if (inferredIsolation.source.kind == IsolationSource::Kind::Explicit) {
5505+
if (inferredIsolation.source.kind == IsolationSource::Kind::Explicit &&
5506+
explicitNonisolatedIsSpecial(nominal)) {
54725507
if (!foundIsolation) {
54735508
// We found an explicitly 'nonisolated' protocol.
54745509
foundIsolation = {
@@ -6088,23 +6123,6 @@ static bool sendableConformanceRequiresNonisolated(NominalTypeDecl *nominal) {
60886123
return requiresNonisolated;
60896124
}
60906125

6091-
/// Determine the default isolation for the given declaration context.
6092-
static DefaultIsolation getDefaultIsolationForContext(const DeclContext *dc) {
6093-
// Check whether there is a file-specific setting.
6094-
if (auto *sourceFile = dc->getParentSourceFile()) {
6095-
if (auto defaultIsolationInFile = sourceFile->getDefaultIsolation())
6096-
return defaultIsolationInFile.value();
6097-
}
6098-
6099-
// If we're in the main module, check the language option.
6100-
ASTContext &ctx = dc->getASTContext();
6101-
if (dc->getParentModule() == ctx.MainModule)
6102-
return ctx.LangOpts.DefaultIsolationBehavior;
6103-
6104-
// Otherwise, default to nonisolated.
6105-
return DefaultIsolation::Nonisolated;
6106-
}
6107-
61086126
/// Determine the default isolation and isolation source for this declaration,
61096127
/// which may still be overridden by other inference rules.
61106128
static std::tuple<InferredActorIsolation, ValueDecl *,
@@ -6300,12 +6318,9 @@ static bool shouldSelfIsolationOverrideDefault(
63006318
if (isa<NominalTypeDecl>(dc))
63016319
return true;
63026320

6303-
// The NoExplicitNonIsolated feature
6304-
if (ctx.LangOpts.hasFeature(Feature::NoExplicitNonIsolated))
6305-
return false;
6306-
6307-
// Suppress when the default isolation is nonisolated.
6308-
return getDefaultIsolationForContext(dc) == DefaultIsolation::Nonisolated;
6321+
/// Only allow explicit nonisolated to override the default when it's
6322+
/// treated as special.
6323+
return explicitNonisolatedIsSpecial(dc);
63096324
}
63106325
}
63116326

test/Concurrency/assume_mainactor.swift

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -227,24 +227,6 @@ actor MyActor2 {
227227
print("123")
228228
}
229229

230-
// https://github.com/swiftlang/swift/issues/82168 - used to fail
231-
nonisolated protocol P {
232-
associatedtype AT
233-
static var at: AT { get }
234-
}
235-
236-
nonisolated struct KP<R: P, V> {
237-
init(keyPath: KeyPath<R, V>) {}
238-
}
239-
240-
struct S: P {
241-
let p: Int
242-
struct AT {
243-
let kp = KP(keyPath: \S.p)
244-
}
245-
static let at = AT() // used to fail here
246-
}
247-
248230
nonisolated func localDeclIsolation() async {
249231
struct Local {
250232
static func f() {}

test/Concurrency/assume_mainactor_typechecker_errors.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,33 @@ extension MyOtherStruct {
116116
// expected-swift6-error@-1{{call to main actor-isolated instance method 'memberOfInt()' in a synchronous nonisolated context}}
117117
}
118118
}
119+
120+
nonisolated protocol P {
121+
func g()
122+
}
123+
124+
struct MyP: P {
125+
func g() {
126+
17.memberOfInt() // okay, on main actor
127+
}
128+
}
129+
130+
// https://github.com/swiftlang/swift/issues/82168 -
131+
nonisolated protocol OtherP {
132+
associatedtype AT
133+
static var at: AT { get }
134+
}
135+
136+
nonisolated struct KP<R: OtherP, V> {
137+
init(keyPath: KeyPath<R, V>) {}
138+
}
139+
140+
struct S: OtherP {
141+
let p: Int
142+
struct AT {
143+
let kp = KP(keyPath: \S.p)
144+
}
145+
146+
// FIXME: This should not be an error.
147+
static let at = AT() // expected-swift6-error{{'S.AT' cannot be constructed because it has no accessible initializers}}
148+
}

test/Concurrency/isolated_conformance_default_actor.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
// REQUIRES: concurrency
44

5+
@MainActor func onMain() { }
6+
57
nonisolated
68
protocol P {
79
func f()
@@ -12,8 +14,14 @@ protocol Q {
1214
func g()
1315
}
1416

17+
// expected-note@+4{{turn data races into runtime errors with '@preconcurrency'}}
18+
// expected-note@+3{{isolate this conformance to the main actor with '@MainActor'}}
19+
// expected-note@+2{{mark all declarations used in the conformance 'nonisolated'}}
20+
// expected-error@+1{{conformance of 'CImplicitMainActorNonisolatedConformance' to protocol 'P' crosses into main actor-isolated code and can cause data races}}
1521
class CImplicitMainActorNonisolatedConformance: nonisolated P {
16-
func f() { } // error: explicitly nonisolated conformance
22+
func f() { // expected-note{{main actor-isolated instance method 'f()' cannot satisfy nonisolated requirement}}
23+
onMain() // okay, f is on @MainActor
24+
}
1725
}
1826

1927

@@ -73,9 +81,6 @@ func acceptSendablePMeta<T: Sendable & P>(_: T.Type) { }
7381
func acceptSendableQMeta<T: Sendable & Q>(_: T.Type) { }
7482

7583
nonisolated func testConformancesFromNonisolated() {
76-
let _: any P = CExplicitMainActor() // okay
77-
let _: any P = CImplicitMainActor() // okay
78-
7984
let _: any P = CNonIsolated()
8085
let _: any P = CImplicitMainActorNonisolatedConformance()
8186

@@ -84,6 +89,10 @@ nonisolated func testConformancesFromNonisolated() {
8489
let _: any Q = CImplicitMainActor()
8590

8691
// Error, these are main-actor-isolated conformances
92+
let _: any P = CExplicitMainActor() // expected-error{{main actor-isolated conformance of 'CExplicitMainActor' to 'P' cannot be used in nonisolated context}}
93+
let _: any P = CImplicitMainActor() // expected-error{{main actor-isolated conformance of 'CImplicitMainActor' to 'P' cannot be used in nonisolated context}}
94+
95+
8796
let _: any Equatable.Type = EquatableStruct.self // expected-error{{main actor-isolated conformance of 'EquatableStruct' to 'Equatable' cannot be used in nonisolated context}}
8897
let _: any Hashable.Type = HashableStruct.self // expected-error{{main actor-isolated conformance of 'HashableStruct' to 'Hashable' cannot be used in nonisolated context}}
8998
let _: any RawRepresentable.Type = RawRepresentableEnum.self

0 commit comments

Comments
 (0)