Skip to content

Commit cd99fb5

Browse files
committed
[Diagnostics] Remove unhelpful notes from witness-isolation diagnostics
When diagnosing an isolation mismatch between a requirement and witness, we would produce notes on the requirement itself suggesting the addition of `async`. This is almost never what you want to do, and is often so far away from the actual conforming type as to be useless. Remove this note, and the non-function fallback that just points at the requirement, because they are unhelpful. This is staging for a rework of the way we deal with conformance-level actor isolation problems.
1 parent 9570e1e commit cd99fb5

14 files changed

+18
-97
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ NOTE(opaque_return_type_declared_here,none,
4242
"opaque return type declared here", ())
4343
NOTE(default_value_declared_here,none,
4444
"default value declared here", ())
45-
NOTE(protocol_requirement_declared_here,none,
46-
"requirement %0 declared here", (const ValueDecl *))
4745

4846
//------------------------------------------------------------------------------
4947
// MARK: Constraint solver diagnostics
@@ -5440,10 +5438,6 @@ NOTE(note_add_async_to_function,none,
54405438
NOTE(note_add_nonisolated_to_decl,none,
54415439
"add 'nonisolated' to %0 to make this %kindonly0 not isolated to the actor",
54425440
(const ValueDecl *))
5443-
NOTE(note_add_async_and_throws_to_decl,none,
5444-
"mark the protocol requirement %0 '%select{|async|throws|async throws}1' "
5445-
"to allow actor-isolated conformances",
5446-
(const ValueDecl *, unsigned))
54475441
NOTE(note_add_distributed_to_decl,none,
54485442
"add 'distributed' to %0 to make this %kindonly0 satisfy the protocol requirement",
54495443
(const ValueDecl *))

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3599,64 +3599,6 @@ ConformanceChecker::checkActorIsolation(ValueDecl *requirement,
35993599
}
36003600
}
36013601

3602-
// If there are remaining options, they are missing async/throws on the
3603-
// requirement itself. If we have a source location for the requirement,
3604-
// provide those in a note.
3605-
3606-
if (requirement->getLoc().isInvalid()) {
3607-
return std::nullopt;
3608-
}
3609-
3610-
if (missingOptions && isa<AbstractFunctionDecl>(requirement)) {
3611-
int suggestAddingModifiers = 0;
3612-
std::string modifiers;
3613-
if (missingOptions.contains(MissingFlags::RequirementAsync)) {
3614-
suggestAddingModifiers += 1;
3615-
modifiers += "async ";
3616-
}
3617-
3618-
if (missingOptions.contains(MissingFlags::RequirementThrows)) {
3619-
suggestAddingModifiers += 2;
3620-
modifiers += "throws ";
3621-
}
3622-
3623-
auto diag = requirement->diagnose(diag::note_add_async_and_throws_to_decl,
3624-
witness, suggestAddingModifiers);
3625-
3626-
// Figure out where to insert the modifiers so we can emit a Fix-It.
3627-
SourceLoc insertLoc;
3628-
bool insertAfter = false;
3629-
if (auto requirementAbstractFunc =
3630-
dyn_cast<AbstractFunctionDecl>(requirement)) {
3631-
if (requirementAbstractFunc->getAsyncLoc().isValid()) {
3632-
// Insert before the "async" (we only have throws).
3633-
insertLoc = requirementAbstractFunc->getAsyncLoc();
3634-
insertAfter = true;
3635-
modifiers = " throws";
3636-
} else if (requirementAbstractFunc->getThrowsLoc().isValid()) {
3637-
// Insert before the "throws" (we only have async)".
3638-
insertLoc = requirementAbstractFunc->getThrowsLoc();
3639-
}
3640-
3641-
// Insert after the parentheses.
3642-
if (insertLoc.isInvalid()) {
3643-
insertLoc = requirementAbstractFunc->getParameters()->getRParenLoc();
3644-
insertAfter = true;
3645-
modifiers = " " + modifiers.substr(0, modifiers.size() - 1);
3646-
}
3647-
}
3648-
3649-
if (insertLoc.isValid()) {
3650-
if (insertAfter)
3651-
diag.fixItInsertAfter(insertLoc, modifiers);
3652-
else
3653-
diag.fixItInsert(insertLoc, modifiers);
3654-
}
3655-
} else {
3656-
requirement->diagnose(diag::protocol_requirement_declared_here,
3657-
requirement);
3658-
}
3659-
36603602
return std::nullopt;
36613603
}
36623604

test/Concurrency/actor_isolation.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1504,7 +1504,7 @@ class MA {
15041504
@SomeGlobalActor class SGA: MA {} // expected-error {{global actor 'SomeGlobalActor'-isolated class 'SGA' has different actor isolation from main actor-isolated superclass 'MA'}}
15051505

15061506
protocol SGA_Proto {
1507-
@SomeGlobalActor func method() // expected-note {{mark the protocol requirement 'method()' 'async' to allow actor-isolated conformances}}
1507+
@SomeGlobalActor func method()
15081508
}
15091509

15101510
// try to override a MA method with inferred isolation from a protocol requirement
@@ -1614,7 +1614,7 @@ class OverridesNonsiolatedInit: SuperWithNonisolatedInit {
16141614
class NonSendable {}
16151615

16161616
protocol NonisolatedProtocol {
1617-
var ns: NonSendable { get } // expected-note {{requirement 'ns' declared here}}
1617+
var ns: NonSendable { get }
16181618
}
16191619

16201620
actor ActorWithNonSendableLet: NonisolatedProtocol {

test/Concurrency/actor_isolation_unsafe.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ actor SomeGlobalActor {
2323
// ----------------------------------------------------------------------
2424
protocol P1 {
2525
// expected-warning@+1 {{'(unsafe)' global actors are deprecated; use '@preconcurrency' instead}}
26-
@MainActor(unsafe) func onMainActor() // expected-note 2{{mark the protocol requirement 'onMainActor()' 'async' to allow actor-isolated conformances}}
26+
@MainActor(unsafe) func onMainActor()
2727
}
2828

2929
struct S1_P1: P1 {

test/Concurrency/global_actor_inference.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func testNotAllInP1(nap1: NotAllInP1) { // expected-note{{add '@SomeGlobalActor'
120120
// Make sure we don't infer 'nonisolated' for stored properties.
121121
@MainActor
122122
protocol Interface {
123-
nonisolated var baz: Int { get } // expected-note{{requirement 'baz' declared here}}
123+
nonisolated var baz: Int { get }
124124
}
125125

126126
@MainActor

test/Concurrency/global_actor_inference_swift6.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ struct S: InferMainActor {
155155
}
156156

157157
protocol InferMainActorInherited: InferMainActor {
158-
func f() // expected-note{{mark the protocol requirement 'f()' 'async' to allow actor-isolated conformances}}
158+
func f()
159159
func g()
160160
}
161161

@@ -198,7 +198,6 @@ class C1: MainActorSuperclass, InferMainFromSuperclass {
198198

199199
protocol InferenceConflictWithSuperclass: MainActorSuperclass, InferSomeGlobalActor {
200200
func g()
201-
// expected-note@-1 {{mark the protocol requirement 'g()' 'async' to allow actor-isolated conformances}}
202201
}
203202

204203

test/Concurrency/isolated_conformance.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// REQUIRES: concurrency
55

66
protocol P {
7-
func f() // expected-note{{mark the protocol requirement 'f()' 'async' to allow actor-isolated conformances}}
7+
func f()
88
}
99

1010
// ----------------------------------------------------------------------------

test/Concurrency/isolated_conformance_default_actor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
nonisolated
88
protocol P {
9-
func f() // expected-note{{mark the protocol requirement 'f()' 'async' to allow actor-isolated conformances}}
9+
func f()
1010
}
1111

1212
@MainActor

test/Concurrency/preconcurrency_conformances.swift

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ extension MyActor : @preconcurrency TestSendability {
9696

9797
protocol Initializable {
9898
init()
99-
// expected-note@-1{{mark the protocol requirement 'init()' 'async' to allow actor-isolated conformances}}
10099
}
101100

102101
final class K : @preconcurrency Initializable {
@@ -130,10 +129,8 @@ struct GlobalActor {
130129
protocol WithIndividuallyIsolatedRequirements {
131130
@MainActor var a: Int { get set }
132131
@GlobalActor var b: Int { get set }
133-
// expected-note@-1 {{requirement 'b' declared here}}
134132

135133
@GlobalActor func test()
136-
// expected-note@-1 {{mark the protocol requirement 'test()' 'async' to allow actor-isolated conformances}}
137134
}
138135

139136
do {
@@ -158,9 +155,7 @@ do {
158155
@MainActor
159156
protocol WithNonIsolated {
160157
var prop: Int { get set }
161-
// expected-note@-1 {{requirement 'prop' declared here}}
162158
nonisolated func test()
163-
// expected-note@-1 {{mark the protocol requirement 'test()' 'async' to allow actor-isolated conformances}}
164159
}
165160

166161
do {
@@ -212,7 +207,7 @@ do {
212207
do {
213208
protocol P1 {}
214209
protocol P2 {
215-
func foo() // expected-note 2 {{mark the protocol requirement 'foo()' 'async' to allow actor-isolated conformances}}
210+
func foo()
216211
}
217212
protocol P3: P1, P2 {}
218213

@@ -278,7 +273,7 @@ do {
278273
do {
279274
protocol P1 {}
280275
protocol P2 {
281-
func foo() // expected-note {{mark the protocol requirement 'foo()' 'async' to allow actor-isolated conformances}}
276+
func foo()
282277
}
283278
protocol P3: P1, P2 {}
284279
protocol P5: P3 {}

test/Concurrency/predates_concurrency.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,6 @@ nonisolated func blah() {
227227

228228
protocol NotIsolated {
229229
func requirement()
230-
// expected-complete-tns-note@-1 {{mark the protocol requirement 'requirement()' 'async' to allow actor-isolated conformances}}
231230
}
232231

233232
extension MainActorPreconcurrency: NotIsolated {

0 commit comments

Comments
 (0)