Skip to content

Commit c18d2b1

Browse files
DougGregorDougGregor
authored
Merge pull request #73741 from DougGregor/suggest-preconcurrency-conformances
Suggest `@preconcurrency` on conformances it could help
2 parents 0b9c25d + 2c0914c commit c18d2b1

21 files changed

+111
-55
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2664,6 +2664,8 @@ WARNING(add_predates_concurrency_import,none,
26642664
"%select{| as warnings}0", (bool, Identifier))
26652665
WARNING(remove_predates_concurrency_import,none,
26662666
"'@preconcurrency' attribute on module %0 has no effect", (Identifier))
2667+
NOTE(add_preconcurrency_to_conformance,none,
2668+
"add '@preconcurrency' to the %0 conformance to defer isolation checking to run time", (DeclName))
26672669
WARNING(remove_public_import,none,
26682670
"public import of %0 was not used in public declarations or inlinable code",
26692671
(const ModuleDecl *))
@@ -5624,12 +5626,12 @@ ERROR(shared_immutable_state_decl,none,
56245626
"shared mutable state",
56255627
(Type, const ValueDecl *))
56265628
NOTE(shared_state_make_immutable,none,
5627-
"convert %0 to a 'let' constant to make the shared state immutable",
5629+
"convert %0 to a 'let' constant to make 'Sendable' shared state immutable",
56285630
(const ValueDecl *))
56295631
NOTE(shared_state_main_actor_node,none,
5630-
"restrict %0 to the main actor if it will only be accessed from the main thread", (const ValueDecl *))
5632+
"annotate %0 with '@MainActor' if property should only be accessed from the main actor", (const ValueDecl *))
56315633
NOTE(shared_state_nonisolated_unsafe,none,
5632-
"unsafely mark %0 as concurrency-safe if all accesses are protected by an external synchronization mechanism", (const ValueDecl *))
5634+
"disable concurrency-safety checks if accesses are protected by an external synchronization mechanism", (const ValueDecl *))
56335635
ERROR(actor_isolated_witness,none,
56345636
"%select{|distributed }0%1 %kind2 cannot be used to satisfy %3 protocol "
56355637
"requirement",

‎include/swift/AST/ProtocolConformance.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,9 @@ class NormalProtocolConformance : public RootProtocolConformance,
530530
/// The location of this protocol conformance in the source.
531531
SourceLoc Loc;
532532

533+
/// The location of the protocol name within the conformance.
534+
SourceLoc ProtocolNameLoc;
535+
533536
/// The location of the `@preconcurrency` attribute, if any.
534537
SourceLoc PreconcurrencyLoc;
535538

@@ -569,10 +572,13 @@ class NormalProtocolConformance : public RootProtocolConformance,
569572
SourceLoc preconcurrencyLoc)
570573
: RootProtocolConformance(ProtocolConformanceKind::Normal,
571574
conformingType),
572-
Protocol(protocol), Loc(loc), PreconcurrencyLoc(preconcurrencyLoc),
575+
Protocol(protocol), Loc(extractNearestSourceLoc(dc)),
576+
ProtocolNameLoc(loc), PreconcurrencyLoc(preconcurrencyLoc),
573577
Context(dc) {
574578
assert(!conformingType->hasArchetype() &&
575579
"ProtocolConformances should store interface types");
580+
assert((preconcurrencyLoc.isInvalid() || isPreconcurrency) &&
581+
"Cannot have a @preconcurrency location without isPreconcurrency");
576582
setState(state);
577583
Bits.NormalProtocolConformance.IsInvalid = false;
578584
Bits.NormalProtocolConformance.IsUnchecked = isUnchecked;
@@ -588,6 +594,9 @@ class NormalProtocolConformance : public RootProtocolConformance,
588594
/// Retrieve the location of this conformance.
589595
SourceLoc getLoc() const { return Loc; }
590596

597+
/// Retrieve the name of the protocol location.
598+
SourceLoc getProtocolNameLoc() const { return ProtocolNameLoc; }
599+
591600
/// Get the declaration context that contains the conforming extension or
592601
/// nominal type declaration.
593602
DeclContext *getDeclContext() const { return Context; }

‎lib/AST/ConformanceLookupTable.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -969,10 +969,11 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
969969
assert(!isa<ProtocolDecl>(conformingDC->getSelfNominalTypeDecl()));
970970
Type conformingType = conformingDC->getSelfInterfaceType();
971971

972-
SourceLoc conformanceLoc
973-
= conformingNominal == conformingDC
974-
? conformingNominal->getLoc()
975-
: cast<ExtensionDecl>(conformingDC)->getLoc();
972+
SourceLoc conformanceLoc =
973+
entry->getLoc().isValid() ? entry->getLoc()
974+
: (conformingNominal == conformingDC
975+
? conformingNominal->getLoc()
976+
: cast<ExtensionDecl>(conformingDC)->getLoc());
976977

977978
NormalProtocolConformance *implyingConf = nullptr;
978979
if (entry->Source.getKind() == ConformanceEntryKind::Implied) {

‎lib/Sema/TypeCheckProtocol.cpp

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,7 +2063,7 @@ static void diagnoseConformanceImpliedByConditionalConformance(
20632063
auto proto = conformance->getProtocol();
20642064
Type protoType = proto->getDeclaredInterfaceType();
20652065
auto implyingProto = implyingConf->getProtocol()->getDeclaredInterfaceType();
2066-
auto loc = implyingConf->getLoc();
2066+
auto loc = extractNearestSourceLoc(implyingConf->getDeclContext());
20672067
Diags.diagnose(loc, diag::conditional_conformances_cannot_imply_conformances,
20682068
conformance->getType(), implyingProto, protoType);
20692069

@@ -3297,13 +3297,28 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
32973297
missingOptions -= MissingFlags::WitnessDistributed;
32983298
}
32993299

3300-
// One way to address the issue is to make the witness function nonisolated.
3301-
if ((isa<AbstractFunctionDecl>(witness) || isa<SubscriptDecl>(witness)) &&
3302-
!hasExplicitGlobalActorAttr(witness) &&
3303-
!isDistributedDecl(requirement) &&
3304-
!isDistributedDecl(witness)) {
3305-
witness->diagnose(diag::note_add_nonisolated_to_decl, witness)
3306-
.fixItInsert(witness->getAttributeInsertionLoc(true), "nonisolated ");
3300+
// If 'nonisolated' or 'preconcurrency' might help us, provide those as
3301+
// options.
3302+
if (!isDistributedDecl(requirement) && !isDistributedDecl(witness)) {
3303+
// One way to address the issue is to make the witness function nonisolated.
3304+
if ((isa<AbstractFunctionDecl>(witness) || isa<SubscriptDecl>(witness)) &&
3305+
!hasExplicitGlobalActorAttr(witness)) {
3306+
witness->diagnose(diag::note_add_nonisolated_to_decl, witness)
3307+
.fixItInsert(witness->getAttributeInsertionLoc(true), "nonisolated ");
3308+
}
3309+
3310+
// Another way to address the issue is to mark the conformance as
3311+
// "preconcurrency".
3312+
if (Conformance->getSourceKind() == ConformanceEntryKind::Explicit &&
3313+
!Conformance->isPreconcurrency() &&
3314+
!suggestedPreconcurrency &&
3315+
!requirementIsolation.isActorIsolated()) {
3316+
Context.Diags.diagnose(Conformance->getProtocolNameLoc(),
3317+
diag::add_preconcurrency_to_conformance,
3318+
Proto->getName())
3319+
.fixItInsert(Conformance->getProtocolNameLoc(), "@preconcurrency ");
3320+
suggestedPreconcurrency = true;
3321+
}
33073322
}
33083323

33093324
// If there are remaining options, they are missing async/throws on the
@@ -5126,9 +5141,8 @@ void ConformanceChecker::resolveValueWitnesses() {
51265141

51275142
SourceLoc preconcurrencyLoc = Conformance->getPreconcurrencyLoc();
51285143
if (preconcurrencyLoc.isValid()) {
5129-
SourceLoc endLoc =
5130-
preconcurrencyLoc.getAdvancedLoc(strlen("@preconcurrency "));
5131-
diag.fixItRemoveChars(preconcurrencyLoc, endLoc);
5144+
SourceLoc endLoc = preconcurrencyLoc.getAdvancedLoc(1);
5145+
diag.fixItRemove(SourceRange(preconcurrencyLoc, endLoc));
51325146
}
51335147
}
51345148

‎lib/Sema/TypeCheckProtocol.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ enum class ResolveWitnessResult {
112112
/// This helper class handles most of the details of checking whether a
113113
/// given type (\c Adoptee) conforms to a protocol (\c Proto).
114114
class ConformanceChecker : public WitnessChecker {
115+
/// Whether we already suggested adding `@preconcurrency`.
116+
bool suggestedPreconcurrency = false;
117+
115118
public:
116119
NormalProtocolConformance *Conformance;
117120
SourceLoc Loc;

‎test/Concurrency/Inputs/sendable_cycle_other.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
struct Foo {
22
static let member = Bar() // expected-complete-warning {{static property 'member' is not concurrency-safe because non-'Sendable' type 'Bar' may have shared mutable state; this is an error in the Swift 6 language mode}}
3-
// expected-complete-note@-1 {{restrict 'member' to the main actor if it will only be accessed from the main thread}}
4-
// expected-complete-note@-2{{unsafely mark 'member' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
3+
// expected-complete-note@-1 {{annotate 'member' with '@MainActor' if property should only be accessed from the main actor}}
4+
// expected-complete-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
55
}

‎test/Concurrency/actor_isolation.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,6 +1586,8 @@ protocol NonisolatedProtocol {
15861586
}
15871587

15881588
actor ActorWithNonSendableLet: NonisolatedProtocol {
1589+
// expected-note@-1{{add '@preconcurrency' to the 'NonisolatedProtocol' conformance to defer isolation checking to run time}}{{32-32=@preconcurrency }}
1590+
15891591
// expected-warning@+1 {{actor-isolated property 'ns' cannot be used to satisfy nonisolated protocol requirement; this is an error in the Swift 6 language mode}}
15901592
let ns = NonSendable()
15911593
}

‎test/Concurrency/concurrency_warnings.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ class GlobalCounter { // expected-note{{class 'GlobalCounter' does not conform t
1313
}
1414

1515
let rs = GlobalCounter() // expected-warning {{let 'rs' is not concurrency-safe because non-'Sendable' type 'GlobalCounter' may have shared mutable state; this is an error in the Swift 6 language mode}}
16-
// expected-note@-1 {{restrict 'rs' to the main actor if it will only be accessed from the main thread}}
17-
// expected-note@-2 {{unsafely mark 'rs' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
16+
// expected-note@-1 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
17+
// expected-note@-2 {{annotate 'rs' with '@MainActor' if property should only be accessed from the main actor}}
1818

1919
import GlobalVariables
2020

‎test/Concurrency/concurrent_value_checking.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,9 @@ typealias BadGenericCF<T> = @Sendable () -> T?
274274
typealias GoodGenericCF<T: Sendable> = @Sendable () -> T? // okay
275275

276276
var concurrentFuncVar: (@Sendable (NotConcurrent) -> Void)? = nil // expected-warning{{var 'concurrentFuncVar' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in the Swift 6 language mode}}
277-
// expected-note@-1 {{restrict 'concurrentFuncVar' to the main actor if it will only be accessed from the main thread}}
278-
// expected-note@-2 {{unsafely mark 'concurrentFuncVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
279-
// expected-note@-3 {{convert 'concurrentFuncVar' to a 'let' constant to make the shared state immutable}}
277+
// expected-note@-1 {{annotate 'concurrentFuncVar' with '@MainActor' if property should only be accessed from the main actor}}
278+
// expected-note@-2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
279+
// expected-note@-3 {{convert 'concurrentFuncVar' to a 'let' constant to make 'Sendable' shared state immutable}}
280280

281281
// ----------------------------------------------------------------------
282282
// Sendable restriction on @Sendable closures.

‎test/Concurrency/flow_isolation.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,9 @@ struct CardboardBox<T> {
520520

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}}
523-
// expected-note@-1 {{restrict 'globalVar' to the main actor if it will only be accessed from the main thread}}
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}}
523+
// expected-note@-1 {{annotate 'globalVar' with '@MainActor' if property should only be accessed from the main actor}}
524+
// expected-note@-2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
525+
// expected-note@-3 {{convert 'globalVar' to a 'let' constant to make 'Sendable' shared state immutable}}
526526

527527
@available(SwiftStdlib 5.1, *)
528528
actor EscapeArtist {

‎test/Concurrency/freestanding_top_level.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
// RUN: %target-swift-frontend -concurrency-model=task-to-thread -typecheck -verify -verify-additional-prefix complete- -strict-concurrency=complete %s
33

44
// 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 }}
6-
// expected-complete-note@+2 {{unsafely mark 'global' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
7-
// expected-complete-note@+1 {{convert 'global' to a 'let' constant to make the shared state immutable}}{{1-4=let}}
5+
// expected-complete-note@+3 {{annotate 'global' with '@MainActor' if property should only be accessed from the main actor}}{{1-1=@MainActor }}
6+
// expected-complete-note@+2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
7+
// expected-complete-note@+1 {{convert 'global' to a 'let' constant to make 'Sendable' shared state immutable}}{{1-4=let}}
88
var global = 10
99

1010
// No warning because we're in the same module.

‎test/Concurrency/global_actor_inference.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ protocol Interface {
117117

118118
@MainActor
119119
class Object: Interface {
120+
// expected-note@-1{{add '@preconcurrency' to the 'Interface' conformance to defer isolation checking to run time}}{{15-15=@preconcurrency }}
121+
120122
var baz: Int = 42 // expected-warning{{main actor-isolated property 'baz' cannot be used to satisfy nonisolated protocol requirement}}
121123
}
122124

‎test/Concurrency/global_variables.swift

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ actor TestGlobalActor {
1515
var mutableIsolatedGlobal = 1
1616

1717
var mutableNonisolatedGlobal = 1 // expected-error{{var 'mutableNonisolatedGlobal' is not concurrency-safe because it is non-isolated global shared mutable state}}
18-
// expected-note@-1{{restrict 'mutableNonisolatedGlobal' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
19-
// expected-note@-2{{unsafely mark 'mutableNonisolatedGlobal' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
20-
// expected-note@-3{{convert 'mutableNonisolatedGlobal' to a 'let' constant to make the shared state immutable}}{{1-4=let}}
18+
// expected-note@-1{{annotate 'mutableNonisolatedGlobal' with '@MainActor' if property should only be accessed from the main actor}}{{1-1=@MainActor }}
19+
// expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
20+
// expected-note@-3{{convert 'mutableNonisolatedGlobal' to a 'let' constant to make 'Sendable' shared state immutable}}{{1-4=let}}
2121

2222
let immutableGlobal = 1
2323

@@ -48,25 +48,25 @@ actor TestActor {
4848
struct TestStatics {
4949
static let immutableExplicitSendable = TestSendable()
5050
static let immutableNonsendable = TestNonsendable() // expected-error{{static property 'immutableNonsendable' is not concurrency-safe because non-'Sendable' type 'TestNonsendable' may have shared mutable state}}
51-
// expected-note@-1 {{restrict 'immutableNonsendable' to the main actor if it will only be accessed from the main thread}}
52-
// expected-note@-2 {{unsafely mark 'immutableNonsendable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
51+
// expected-note@-1 {{annotate 'immutableNonsendable' with '@MainActor' if property should only be accessed from the main actor}}
52+
// expected-note@-2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
5353
static nonisolated(unsafe) let immutableNonisolatedUnsafe = TestNonsendable()
5454
static nonisolated let immutableNonisolated = TestNonsendable() // expected-error{{static property 'immutableNonisolated' is not concurrency-safe because non-'Sendable' type 'TestNonsendable' may have shared mutable state}}
55-
// expected-note@-1 {{restrict 'immutableNonisolated' to the main actor if it will only be accessed from the main thread}}
55+
// expected-note@-1 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
5656
// expected-error@-2 {{'nonisolated' can not be applied to variable with non-'Sendable' type 'TestNonsendable'}}
57-
// expected-note@-3{{unsafely mark 'immutableNonisolated' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
57+
// expected-note@-3{{annotate 'immutableNonisolated' with '@MainActor' if property should only be accessed from the main actor}}
5858
static nonisolated(unsafe) let immutableNonisolatedUnsafeSendable = TestSendable()
5959
// expected-warning@-1 {{'nonisolated(unsafe)' is unnecessary for a constant with 'Sendable' type 'TestSendable', consider removing it}} {{10-30=}}
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}}
62-
// expected-note@-1{{convert 'mutable' to a 'let' constant to make the shared state immutable}}
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}}
62+
// expected-note@-1{{convert 'mutable' to a 'let' constant to make 'Sendable' shared state immutable}}
63+
// expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
64+
// expected-note@-3{{annotate 'mutable' with '@MainActor' if property should only be accessed from the main actor}}
6565
static var computedProperty: Int { 0 } // computed property that, though static, has no storage so is not a global
6666
@TestWrapper static var wrapped: Int // expected-error{{static property 'wrapped' is not concurrency-safe because it is non-isolated global shared mutable state}}
67-
// expected-note@-1{{convert 'wrapped' to a 'let' constant to make the shared state immutable}}{{23-26=let}}
68-
// expected-note@-2{{unsafely mark 'wrapped' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{16-16=nonisolated(unsafe) }}
69-
// expected-note@-3{{restrict 'wrapped' to the main actor if it will only be accessed from the main thread}}{{3-3=@MainActor }}
67+
// expected-note@-1{{convert 'wrapped' to a 'let' constant to make 'Sendable' shared state immutable}}{{23-26=let}}
68+
// expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}{{16-16=nonisolated(unsafe) }}
69+
// expected-note@-3{{annotate 'wrapped' with '@MainActor' if property should only be accessed from the main actor}}{{3-3=@MainActor }}
7070
}
7171

7272
public actor TestPublicActor {

‎test/Concurrency/preconcurrency_conformances.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,21 @@ extension MyActor : @preconcurrency TestSendability {
9191

9292
protocol Initializable {
9393
init()
94+
// expected-note@-1{{mark the protocol requirement 'init()' 'async' to allow actor-isolated conformances}}
9495
}
9596

9697
final class K : @preconcurrency Initializable {
9798
// expected-warning@-1 {{@preconcurrency attribute on conformance to 'Initializable' has no effect}}
9899
init() {} // Ok
99100
}
100101

102+
@MainActor
103+
final class MainActorK: Initializable {
104+
// expected-note@-1{{add '@preconcurrency' to the 'Initializable' conformance to defer isolation checking to run time}}{{25-25=@preconcurrency }}
105+
init() { } // expected-warning{{main actor-isolated initializer 'init()' cannot be used to satisfy nonisolated protocol requirement}}
106+
// expected-note@-1{{add 'nonisolated' to 'init()' to make this initializer not isolated to the actor}}
107+
}
108+
101109
protocol WithAssoc {
102110
associatedtype T
103111
func test() -> T

‎test/Concurrency/predates_concurrency.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ protocol NotIsolated {
230230
}
231231

232232
extension MainActorPreconcurrency: NotIsolated {
233+
// expected-complete-note@-1{{add '@preconcurrency' to the 'NotIsolated' conformance to suppress isolation-related diagnostics}}{{36-36=@preconcurrency }}
234+
// expected-complete-tns-note@-2{{add '@preconcurrency' to the 'NotIsolated' conformance to defer isolation checking to run time}}{{36-36=@preconcurrency }}
235+
233236
func requirement() {}
234237
// expected-complete-tns-warning@-1 {{main actor-isolated instance method 'requirement()' cannot be used to satisfy nonisolated protocol requirement}}
235238
// expected-complete-tns-note@-2 {{add 'nonisolated' to 'requirement()' to make this instance method not isolated to the actor}}

0 commit comments

Comments
 (0)