[Sema] Group isolation errors in checkApply

Author: angela-laar

lib/Sema/TypeCheckConcurrency.cpp

@@ -2003,11 +2003,15 @@ namespace {
     /// an expression or function.
     llvm::SmallDenseMap<const DeclContext *, ActorIsolation> requiredIsolation;
 
-    using IsolationPair = std::pair<ReferencedActor::Kind, ActorIsolation>;
+    using ActorRefKindPair = std::pair<ReferencedActor::Kind, ActorIsolation>;
+
+    using IsolationPair = std::pair<ActorIsolation, ActorIsolation>;
 
     using DiagnosticList = std::vector<IsolationError>;
 
-    llvm::DenseMap<IsolationPair, DiagnosticList> isoErrors;
+    llvm::DenseMap<ActorRefKindPair, DiagnosticList> refErrors;
+
+    llvm::DenseMap<IsolationPair, DiagnosticList> applyErrors;
 
     /// Keeps track of the capture context of variables that have been
     /// explicitly captured in closures.
@@ -2065,8 +2069,8 @@ namespace {
     bool diagnoseIsolationErrors() {
       bool diagnosedError = false;
 
-      for (auto list : isoErrors) {
-        IsolationPair key = list.getFirst();
+      for (auto list : refErrors) {
+        ActorRefKindPair key = list.getFirst();
         DiagnosticList errors = list.getSecond();
         ActorIsolation isolation = key.second;
 
@@ -2087,6 +2091,30 @@ namespace {
               .limitBehavior(behavior);
         }
       }
+
+      for (auto list : applyErrors) {
+        IsolationPair key = list.getFirst();
+        DiagnosticList errors = list.getSecond();
+        ActorIsolation isolation = key.first;
+
+        auto behavior = DiagnosticBehavior::Error;
+
+        // Add Fix-it for missing @SomeActor annotation
+        if (isolation.isGlobalActor()) {
+          if (missingGlobalActorOnContext(
+                                          const_cast<DeclContext*>(getDeclContext()), isolation.getGlobalActor())) {
+            behavior= DiagnosticBehavior::Note;
+          }
+        }
+
+        for (IsolationError error : errors) {
+          // Diagnose actor_isolated_non_self_reference as note
+          // if we provide fix-it in missingGlobalActorOnContext
+          ctx.Diags.diagnose(error.loc, error.diag)
+              .limitBehavior(behavior);
+        }
+      }
+
       return diagnosedError;
     }
 
@@ -2422,8 +2450,6 @@ namespace {
         requiredIsolationLoc = expr->getLoc();
 
       expr->walk(*this);
-      // print all diagnostics here :)
-      //diagnoseIsolationErrors(getDeclContext(), isoErrors);
       requiredIsolationLoc = SourceLoc();
       return requiredIsolation[getDeclContext()];
     }
@@ -3225,28 +3251,49 @@ namespace {
       // If we need to mark the call as implicitly asynchronous, make sure
       // we're in an asynchronous context.
       if (requiresAsync && !getDeclContext()->isAsyncContext()) {
-        if (calleeDecl) {
-          auto preconcurrency = getContextIsolation().preconcurrency() ||
-              calleeDecl->preconcurrency();
-          ctx.Diags.diagnose(
-              apply->getLoc(), diag::actor_isolated_call_decl,
-              *unsatisfiedIsolation,
-              calleeDecl,
-              getContextIsolation())
-            .warnUntilSwiftVersionIf(preconcurrency, 6);
-          calleeDecl->diagnose(diag::actor_isolated_sync_func, calleeDecl);
+
+        if (ctx.LangOpts.hasFeature(Feature::GroupActorErrors)) {
+          IsolationError isoMismatch =
+          IsolationError(
+                         apply->getLoc(),
+                         Diagnostic(diag::actor_isolated_call_decl,
+                                    *unsatisfiedIsolation,
+                                    calleeDecl,
+                                    getContextIsolation()));
+
+            auto iter = applyErrors.find(std::make_pair(*unsatisfiedIsolation, getContextIsolation()));
+            if (iter != applyErrors.end()){
+              iter->second.push_back(isoMismatch);
+            } else {
+              DiagnosticList list;
+              list.push_back(isoMismatch);
+              auto keyPair = std::make_pair(*unsatisfiedIsolation, getContextIsolation());
+              applyErrors.insert(std::make_pair(keyPair, list));
+            }
         } else {
-          ctx.Diags.diagnose(
-              apply->getLoc(), diag::actor_isolated_call, *unsatisfiedIsolation,
-              getContextIsolation())
+          if (calleeDecl) {
+            auto preconcurrency = getContextIsolation().preconcurrency() ||
+            calleeDecl->preconcurrency();
+            ctx.Diags.diagnose(
+                               apply->getLoc(), diag::actor_isolated_call_decl,
+                               *unsatisfiedIsolation,
+                               calleeDecl,
+                               getContextIsolation())
+            .warnUntilSwiftVersionIf(preconcurrency, 6);
+            calleeDecl->diagnose(diag::actor_isolated_sync_func, calleeDecl);
+          } else {
+            ctx.Diags.diagnose(
+                               apply->getLoc(), diag::actor_isolated_call, *unsatisfiedIsolation,
+                               getContextIsolation())
             .warnUntilSwiftVersionIf(getContextIsolation().preconcurrency(), 6);
-        }
+          }
 
-//        if (unsatisfiedIsolation->isGlobalActor()) {
-//          noteGlobalActorOnContext(
-//              const_cast<DeclContext *>(getDeclContext()),
-//              unsatisfiedIsolation->getGlobalActor());
-//        }
+          if (unsatisfiedIsolation->isGlobalActor()) {
+            missingGlobalActorOnContext(
+                                        const_cast<DeclContext *>(getDeclContext()),
+                                        unsatisfiedIsolation->getGlobalActor());
+          }
+        }
 
         return true;
       }
@@ -3622,14 +3669,14 @@ namespace {
                                                                         refKind + 1, refGlobalActor,
                                                                         result.isolation));
 
-            auto iter = isoErrors.find(std::make_pair(refKind,result.isolation));
-            if (iter != isoErrors.end()){
+            auto iter = refErrors.find(std::make_pair(refKind,result.isolation));
+            if (iter != refErrors.end()){
               iter->second.push_back(isoMismatch);
             } else {
               DiagnosticList list;
               list.push_back(isoMismatch);
               auto keyPair = std::make_pair(refKind,result.isolation);
-              isoErrors.insert(std::make_pair(keyPair, list));
+              refErrors.insert(std::make_pair(keyPair, list));
             }
           } else {
             ctx.Diags.diagnose(

lib/Sema/TypeCheckConcurrency.h

@@ -259,8 +259,6 @@ struct IsolationError {
 
 };
 
-static bool diagnoseIsolationErrors(DeclContext *dc, ArrayRef<IsolationError> errors);
-
 /// Individual options used with \c FunctionCheckOptions
 enum class FunctionCheckKind {
   /// Check params

test/Concurrency/grouped_actor_isolation_diagnostics.swift

@@ -0,0 +1,77 @@
+// RUN: %target-typecheck-verify-swift -disable-availability-checking -enable-experimental-feature GroupActorErrors
+// REQUIRES: concurrency
+
+
+@MainActor
+protocol P {
+  func f()
+  nonisolated func g()
+}
+
+struct S_P: P {
+  func f() { }
+  // expected-complete-sns-note @-1 {{calls to instance method 'f()' from outside of its actor context are implicitly asynchronous}}
+  func g() { }
+}
+
+func testP2(x: S_P, p: P) { // expected-error{{add '@MainActor' to make global function 'testP2(x:p:)' part of global actor 'MainActor'}}
+  p.f() // expected-note{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  p.f() // expected-note{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  p.f() // expected-note{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  p.g() // OKAY
+  x.f() // expected-note{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  x.f() // expected-note{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  x.f() // expected-note{{call to main actor-isolated instance method 'f()' in a synchronous nonisolated context}}
+  x.g() // OKAY
+}
+
+actor SomeActor { }
+
+@globalActor
+struct SomeGlobalActor {
+  static let shared = SomeActor()
+}
+
+@propertyWrapper
+struct WrapperOnActor<Wrapped: Sendable> {
+  private var stored: Wrapped
+
+  nonisolated init(wrappedValue: Wrapped) {
+    stored = wrappedValue
+  }
+
+  @MainActor var wrappedValue: Wrapped {
+    get { }
+    set { }
+  }
+
+  @SomeGlobalActor var projectedValue: Wrapped {
+    get {  }
+    set { }
+  }
+}
+
+struct HasWrapperOnActor {
+  @WrapperOnActor var x: Int = 0
+  @WrapperOnActor var y: String = ""
+  @WrapperOnActor var z: (Double, Double) = (1.0,2.0)
+
+  func testWrapped() { // expected-error{{add '@MainActor' to make instance method 'testWrapped()' part of global actor 'MainActor'}}
+    _ = x // expected-note{{main actor-isolated property 'x' can not be referenced from a non-isolated context}}
+    _ = y // expected-note{{main actor-isolated property 'y' can not be referenced from a non-isolated context}}
+    _ = z // expected-note{{main actor-isolated property 'z' can not be referenced from a non-isolated context}}
+  }
+  func testProjected(){ // expected-error{{add '@SomeGlobalActor' to make instance method 'testProjected()' part of global actor 'SomeGlobalActor'}}
+    _ = $x // expected-note{{global actor 'SomeGlobalActor'-isolated property '$x' can not be referenced from a non-isolated context}}
+    _ = $y // expected-note{{global actor 'SomeGlobalActor'-isolated property '$y' can not be referenced from a non-isolated context}}
+    _ = $z // expected-note{{global actor 'SomeGlobalActor'-isolated property '$z' can not be referenced from a non-isolated context}}
+  }
+
+  @MainActor
+  func testMA(){ }
+
+  func testErrors() { // expected-error{{add '@MainActor' to make instance method 'testErrors()' part of global actor 'MainActor'}}
+    testMA() // expected-note{{call to main actor-isolated instance method 'testMA()' in a synchronous nonisolated context}}
+    testMA() // expected-note{{call to main actor-isolated instance method 'testMA()' in a synchronous nonisolated context}}
+  }
+}