Update diagnostic text to address code review feedback

rofle100lvl · May 21, 2024 · 5a0e70a · 5a0e70a
1 parent 640042f
commit 5a0e70a
Show file tree

Hide file tree

Showing 19 changed files with 53 additions and 52 deletions.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -2656,7 +2656,7 @@ WARNING(add_predates_concurrency_import,none,
 WARNING(remove_predates_concurrency_import,none,
      "'@preconcurrency' attribute on module %0 has no effect", (Identifier))
 NOTE(add_preconcurrency_to_conformance,none,
-     "add '@preconcurrency' to the %0 conformance to suppress isolation-related diagnostics", (DeclName))
+     "add '@preconcurrency' to the %0 conformance to defer isolation checking to run time", (DeclName))
 WARNING(remove_public_import,none,
         "public import of %0 was not used in public declarations or inlinable code",
         (const ModuleDecl *))
@@ -5617,12 +5617,12 @@ ERROR(shared_immutable_state_decl,none,
       "shared mutable state",
       (Type, const ValueDecl *))
 NOTE(shared_state_make_immutable,none,
-     "convert %0 to a 'let' constant to make the shared state immutable",
+     "convert %0 to a 'let' constant to make 'Sendable' shared state immutable",
      (const ValueDecl *))
 NOTE(shared_state_main_actor_node,none,
-     "restrict %0 to the main actor if it will only be accessed from the main thread", (const ValueDecl *))
+     "annotate %0 with '@MainActor' if property should only be accessed from the main actor", (const ValueDecl *))
 NOTE(shared_state_nonisolated_unsafe,none,
-     "unsafely mark %0 as concurrency-safe if all accesses are protected by an external synchronization mechanism", (const ValueDecl *))
+     "disable concurrency-safety checks if accesses are protected by an external synchronization mechanism", (const ValueDecl *))
 ERROR(actor_isolated_witness,none,
      "%select{|distributed }0%1 %kind2 cannot be used to satisfy %3 protocol "
      "requirement",

diff --git a/include/swift/AST/ProtocolConformance.h b/include/swift/AST/ProtocolConformance.h
@@ -573,6 +573,8 @@ class NormalProtocolConformance : public RootProtocolConformance,
         Context(dc) {
     assert(!conformingType->hasArchetype() &&
            "ProtocolConformances should store interface types");
+    assert((preconcurrencyLoc.isInvalid() || isPreconcurrency) &&
+           "Cannot have a @preconcurrency location without isPreconcurrency");
     setState(state);
     Bits.NormalProtocolConformance.IsInvalid = false;
     Bits.NormalProtocolConformance.IsUnchecked = isUnchecked;

diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -5141,9 +5141,8 @@ void ConformanceChecker::resolveValueWitnesses() {
 
     SourceLoc preconcurrencyLoc = Conformance->getPreconcurrencyLoc();
     if (preconcurrencyLoc.isValid()) {
-      SourceLoc endLoc =
-          preconcurrencyLoc.getAdvancedLoc(strlen("@preconcurrency "));
-      diag.fixItRemoveChars(preconcurrencyLoc, endLoc);
+      SourceLoc endLoc = preconcurrencyLoc.getAdvancedLoc(1);
+      diag.fixItRemove(SourceRange(preconcurrencyLoc, endLoc));
     }
   }
 

diff --git a/test/Concurrency/Inputs/sendable_cycle_other.swift b/test/Concurrency/Inputs/sendable_cycle_other.swift
@@ -1,5 +1,5 @@
 struct Foo {
   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}}
-  // expected-complete-note@-1 {{restrict 'member' to the main actor if it will only be accessed from the main thread}}
-  // expected-complete-note@-2{{unsafely mark 'member' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+  // expected-complete-note@-1 {{annotate 'member' with '@MainActor' if property should only be accessed from the main actor}}
+  // expected-complete-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
 }
diff --git a/test/Concurrency/actor_isolation.swift b/test/Concurrency/actor_isolation.swift
@@ -1586,7 +1586,7 @@ protocol NonisolatedProtocol {
 }
 
 actor ActorWithNonSendableLet: NonisolatedProtocol {
-  // expected-note@-1{{add '@preconcurrency' to the 'NonisolatedProtocol' conformance to suppress isolation-related diagnostics}}{{32-32=@preconcurrency }}
+  // expected-note@-1{{add '@preconcurrency' to the 'NonisolatedProtocol' conformance to defer isolation checking to run time}}{{32-32=@preconcurrency }}
 
   // 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}}
   let ns = NonSendable()

diff --git a/test/Concurrency/concurrency_warnings.swift b/test/Concurrency/concurrency_warnings.swift
@@ -13,8 +13,8 @@ class GlobalCounter { // expected-note{{class 'GlobalCounter' does not conform t
 }
 
 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}}
-// expected-note@-1 {{restrict 'rs' to the main actor if it will only be accessed from the main thread}}
-// expected-note@-2 {{unsafely mark 'rs' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+// expected-note@-1 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
+// expected-note@-2 {{annotate 'rs' with '@MainActor' if property should only be accessed from the main actor}}
 
 import GlobalVariables
 

diff --git a/test/Concurrency/concurrent_value_checking.swift b/test/Concurrency/concurrent_value_checking.swift
@@ -274,9 +274,9 @@ typealias BadGenericCF<T> = @Sendable () -> T?
 typealias GoodGenericCF<T: Sendable> = @Sendable () -> T? // okay
 
 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}}
-// expected-note@-1 {{restrict 'concurrentFuncVar' to the main actor if it will only be accessed from the main thread}}
-// expected-note@-2 {{unsafely mark 'concurrentFuncVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
-// expected-note@-3 {{convert 'concurrentFuncVar' to a 'let' constant to make the shared state immutable}}
+// expected-note@-1 {{annotate 'concurrentFuncVar' with '@MainActor' if property should only be accessed from the main actor}}
+// expected-note@-2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
+// expected-note@-3 {{convert 'concurrentFuncVar' to a 'let' constant to make 'Sendable' shared state immutable}}
 
 // ----------------------------------------------------------------------
 // Sendable restriction on @Sendable closures.

diff --git a/test/Concurrency/flow_isolation.swift b/test/Concurrency/flow_isolation.swift
@@ -520,9 +520,9 @@ struct CardboardBox<T> {
 
 @available(SwiftStdlib 5.1, *)
 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}}
-// expected-note@-1 {{restrict 'globalVar' to the main actor if it will only be accessed from the main thread}}
-// expected-note@-2 {{unsafely mark 'globalVar' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
-// expected-note@-3 {{convert 'globalVar' to a 'let' constant to make the shared state immutable}}
+// expected-note@-1 {{annotate 'globalVar' with '@MainActor' if property should only be accessed from the main actor}}
+// expected-note@-2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
+// expected-note@-3 {{convert 'globalVar' to a 'let' constant to make 'Sendable' shared state immutable}}
 
 @available(SwiftStdlib 5.1, *)
 actor EscapeArtist {

diff --git a/test/Concurrency/freestanding_top_level.swift b/test/Concurrency/freestanding_top_level.swift
@@ -2,9 +2,9 @@
 // RUN: %target-swift-frontend -concurrency-model=task-to-thread -typecheck  -verify -verify-additional-prefix complete- -strict-concurrency=complete %s
 
 // 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}}
-// expected-complete-note@+3 {{restrict 'global' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
-// expected-complete-note@+2 {{unsafely mark 'global' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
-// expected-complete-note@+1 {{convert 'global' to a 'let' constant to make the shared state immutable}}{{1-4=let}}
+// expected-complete-note@+3 {{annotate 'global' with '@MainActor' if property should only be accessed from the main actor}}{{1-1=@MainActor }}
+// expected-complete-note@+2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
+// expected-complete-note@+1 {{convert 'global' to a 'let' constant to make 'Sendable' shared state immutable}}{{1-4=let}}
 var global = 10
 
 // No warning because we're in the same module.

diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift
@@ -117,7 +117,7 @@ protocol Interface {
 
 @MainActor
 class Object: Interface {
-  // expected-note@-1{{add '@preconcurrency' to the 'Interface' conformance to suppress isolation-related diagnostics}}{{15-15=@preconcurrency }}
+  // expected-note@-1{{add '@preconcurrency' to the 'Interface' conformance to defer isolation checking to run time}}{{15-15=@preconcurrency }}
 
   var baz: Int = 42 // expected-warning{{main actor-isolated property 'baz' cannot be used to satisfy nonisolated protocol requirement}}
 }

diff --git a/test/Concurrency/global_variables.swift b/test/Concurrency/global_variables.swift
@@ -15,9 +15,9 @@ actor TestGlobalActor {
 var mutableIsolatedGlobal = 1
 
 var mutableNonisolatedGlobal = 1 // expected-error{{var 'mutableNonisolatedGlobal' is not concurrency-safe because it is non-isolated global shared mutable state}}
-// expected-note@-1{{restrict 'mutableNonisolatedGlobal' to the main actor if it will only be accessed from the main thread}}{{1-1=@MainActor }}
-// expected-note@-2{{unsafely mark 'mutableNonisolatedGlobal' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
-// expected-note@-3{{convert 'mutableNonisolatedGlobal' to a 'let' constant to make the shared state immutable}}{{1-4=let}}
+// expected-note@-1{{annotate 'mutableNonisolatedGlobal' with '@MainActor' if property should only be accessed from the main actor}}{{1-1=@MainActor }}
+// expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}{{1-1=nonisolated(unsafe) }}
+// expected-note@-3{{convert 'mutableNonisolatedGlobal' to a 'let' constant to make 'Sendable' shared state immutable}}{{1-4=let}}
 
 let immutableGlobal = 1
 
@@ -48,25 +48,25 @@ actor TestActor {
 struct TestStatics {
   static let immutableExplicitSendable = TestSendable()
   static let immutableNonsendable = TestNonsendable() // expected-error{{static property 'immutableNonsendable' is not concurrency-safe because non-'Sendable' type 'TestNonsendable' may have shared mutable state}}
-  // expected-note@-1 {{restrict 'immutableNonsendable' to the main actor if it will only be accessed from the main thread}}
-  // expected-note@-2 {{unsafely mark 'immutableNonsendable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+  // expected-note@-1 {{annotate 'immutableNonsendable' with '@MainActor' if property should only be accessed from the main actor}}
+  // expected-note@-2 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
   static nonisolated(unsafe) let immutableNonisolatedUnsafe = TestNonsendable()
   static nonisolated let immutableNonisolated = TestNonsendable() // expected-error{{static property 'immutableNonisolated' is not concurrency-safe because non-'Sendable' type 'TestNonsendable' may have shared mutable state}}
-  // expected-note@-1 {{restrict 'immutableNonisolated' to the main actor if it will only be accessed from the main thread}}
+  // expected-note@-1 {{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
   // expected-error@-2 {{'nonisolated' can not be applied to variable with non-'Sendable' type 'TestNonsendable'}}
-  // expected-note@-3{{unsafely mark 'immutableNonisolated' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+  // expected-note@-3{{annotate 'immutableNonisolated' with '@MainActor' if property should only be accessed from the main actor}}
   static nonisolated(unsafe) let immutableNonisolatedUnsafeSendable = TestSendable()
   // expected-warning@-1 {{'nonisolated(unsafe)' is unnecessary for a constant with 'Sendable' type 'TestSendable', consider removing it}} {{10-30=}}
   static let immutableInferredSendable = 0
   static var mutable = 0 // expected-error{{static property 'mutable' is not concurrency-safe because it is non-isolated global shared mutable state}}
-  // expected-note@-1{{convert 'mutable' to a 'let' constant to make the shared state immutable}}
-  // expected-note@-2{{unsafely mark 'mutable' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
-  // expected-note@-3{{restrict 'mutable' to the main actor if it will only be accessed from the main thread}}
+  // expected-note@-1{{convert 'mutable' to a 'let' constant to make 'Sendable' shared state immutable}}
+  // expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
+  // expected-note@-3{{annotate 'mutable' with '@MainActor' if property should only be accessed from the main actor}}
   static var computedProperty: Int { 0 } // computed property that, though static, has no storage so is not a global
   @TestWrapper static var wrapped: Int // expected-error{{static property 'wrapped' is not concurrency-safe because it is non-isolated global shared mutable state}}
-  // expected-note@-1{{convert 'wrapped' to a 'let' constant to make the shared state immutable}}{{23-26=let}}
-  // expected-note@-2{{unsafely mark 'wrapped' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}{{16-16=nonisolated(unsafe) }}
-  // expected-note@-3{{restrict 'wrapped' to the main actor if it will only be accessed from the main thread}}{{3-3=@MainActor }}
+  // expected-note@-1{{convert 'wrapped' to a 'let' constant to make 'Sendable' shared state immutable}}{{23-26=let}}
+  // expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}{{16-16=nonisolated(unsafe) }}
+  // expected-note@-3{{annotate 'wrapped' with '@MainActor' if property should only be accessed from the main actor}}{{3-3=@MainActor }}
 }
 
 public actor TestPublicActor {

diff --git a/test/Concurrency/preconcurrency_conformances.swift b/test/Concurrency/preconcurrency_conformances.swift
@@ -101,7 +101,7 @@ final class K : @preconcurrency Initializable {
 
 @MainActor
 final class MainActorK: Initializable {
-  // expected-note@-1{{add '@preconcurrency' to the 'Initializable' conformance to suppress isolation-related diagnostics}}{{25-25=@preconcurrency }}
+  // expected-note@-1{{add '@preconcurrency' to the 'Initializable' conformance to defer isolation checking to run time}}{{25-25=@preconcurrency }}
   init() { } // expected-warning{{main actor-isolated initializer 'init()' cannot be used to satisfy nonisolated protocol requirement}}
   // expected-note@-1{{add 'nonisolated' to 'init()' to make this initializer not isolated to the actor}}
 }

diff --git a/test/Concurrency/predates_concurrency.swift b/test/Concurrency/predates_concurrency.swift
@@ -231,7 +231,7 @@ protocol NotIsolated {
 
 extension MainActorPreconcurrency: NotIsolated {
   // expected-complete-note@-1{{add '@preconcurrency' to the 'NotIsolated' conformance to suppress isolation-related diagnostics}}{{36-36=@preconcurrency }}
-  // expected-complete-tns-note@-2{{add '@preconcurrency' to the 'NotIsolated' conformance to suppress isolation-related diagnostics}}{{36-36=@preconcurrency }}
+  // expected-complete-tns-note@-2{{add '@preconcurrency' to the 'NotIsolated' conformance to defer isolation checking to run time}}{{36-36=@preconcurrency }}
 
   func requirement() {}
   // expected-complete-tns-warning@-1 {{main actor-isolated instance method 'requirement()' cannot be used to satisfy nonisolated protocol requirement}}

diff --git a/test/Concurrency/predates_concurrency_import.swift b/test/Concurrency/predates_concurrency_import.swift
@@ -44,8 +44,8 @@ func test(
 let nonStrictGlobal = NonStrictClass() // no warning
 
 let strictGlobal = StrictStruct() // expected-warning{{let 'strictGlobal' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
-// expected-note@-1{{restrict 'strictGlobal' to the main actor if it will only be accessed from the main thread}}
-// expected-note@-2{{unsafely mark 'strictGlobal' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+// expected-note@-1{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
+// expected-note@-2{{annotate 'strictGlobal' with '@MainActor' if property should only be accessed from the main actor}}
 
 extension NonStrictClass {
   @Sendable func f() { }
@@ -62,8 +62,8 @@ struct HasStatics {
   nonisolated static let ss: StrictStruct = StrictStruct()
   // expected-warning@-1{{'nonisolated' can not be applied to variable with non-'Sendable' type 'StrictStruct'}}
   // expected-warning@-2{{static property 'ss' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
-  // expected-note@-3{{restrict 'ss' to the main actor if it will only be accessed from the main thread}}
-  // expected-note@-4{{unsafely mark 'ss' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+  // expected-note@-3{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
+  // expected-note@-4{{annotate 'ss' with '@MainActor' if property should only be accessed from the main actor}}
 }
 
 extension NonStrictClass2: @retroactive MySendableProto { }

diff --git a/test/Concurrency/predates_concurrency_import_swift6.swift b/test/Concurrency/predates_concurrency_import_swift6.swift
@@ -18,8 +18,8 @@ func test(ss: StrictStruct, ns: NonStrictClass) {
 
 let nonStrictGlobal = NonStrictClass()
 let strictGlobal = StrictStruct() // expected-warning{{let 'strictGlobal' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
-// expected-note@-1{{restrict 'strictGlobal' to the main actor if it will only be accessed from the main thread}}
-// expected-note@-2{{unsafely mark 'strictGlobal' as concurrency-safe if all accesses are protected by an external synchronization mechanism}}
+// expected-note@-1{{annotate 'strictGlobal' with '@MainActor' if property should only be accessed from the main actor}}
+// expected-note@-2{{disable concurrency-safety checks if accesses are protected by an external synchronization mechanism}}
 
 extension NonStrictClass {
   @Sendable func f() { }

diff --git a/test/Distributed/distributed_protocol_isolation.swift b/test/Distributed/distributed_protocol_isolation.swift
@@ -106,7 +106,7 @@ protocol StrictlyLocal {
 }
 
 distributed actor Nope1_StrictlyLocal: StrictlyLocal {
-  // expected-note@-1{{add '@preconcurrency' to the 'StrictlyLocal' conformance to suppress isolation-related diagnostics}}
+  // expected-note@-1{{add '@preconcurrency' to the 'StrictlyLocal' conformance to defer isolation checking to run time}}
 
   func local() {}
   // expected-error@-1{{distributed actor-isolated instance method 'local()' cannot be used to satisfy nonisolated protocol requirement}}
@@ -159,7 +159,7 @@ actor LocalOK_ImplicitlyThrowsAsync_AsyncThrowsAll: AsyncThrowsAll {
 }
 
 distributed actor Nope1_AsyncThrowsAll: AsyncThrowsAll {
-  // expected-note@-1{{add '@preconcurrency' to the 'AsyncThrowsAll' conformance to suppress isolation-related diagnostics}}
+  // expected-note@-1{{add '@preconcurrency' to the 'AsyncThrowsAll' conformance to defer isolation checking to run time}}
 
   func maybe(param: String, int: Int) async throws -> Int { 111 }
   // expected-error@-1{{distributed actor-isolated instance method 'maybe(param:int:)' cannot be used to satisfy nonisolated protocol requirement}}
@@ -206,7 +206,7 @@ func test_watching_A(a: A_TerminationWatchingA) async throws {
 }
 
 distributed actor DA_TerminationWatchingA: TerminationWatchingA {
-  // expected-note@-1{{add '@preconcurrency' to the 'TerminationWatchingA' conformance to suppress isolation-related diagnostics}}
+  // expected-note@-1{{add '@preconcurrency' to the 'TerminationWatchingA' conformance to defer isolation checking to run time}}
 
   func terminated(a: String) { }
   // expected-error@-1{{distributed actor-isolated instance method 'terminated(a:)' cannot be used to satisfy nonisolated protocol requirement}}