Merge pull request swiftlang#36345 from DougGregor/cross-actor-global-actor-ref

Author: DougGregor

lib/Sema/TypeCheckConcurrency.cpp

@@ -1724,61 +1724,36 @@ namespace {
       }
 
       case ActorIsolation::Unspecified: {
-        // NOTE: we must always inspect for implicitlyAsync
         auto result = tryMarkImplicitlyAsync(loc, valueRef, context);
-        bool implicitlyAsyncExpr = (result == AsyncMarkingResult::FoundAsync);
-        bool didEmitDiagnostic = false;
+        if (result == AsyncMarkingResult::FoundAsync)
+          return false;
 
-        auto emitError = [&](bool justNote = false) {
-          didEmitDiagnostic = true;
-          if (!justNote) {
-            auto useKind = static_cast<unsigned>(
-                kindOfUsage(value, context).getValueOr(VarRefUseEnv::Read));
-            ctx.Diags.diagnose(
-              loc, diag::global_actor_from_nonactor_context,
-              value->getDescriptiveKind(), value->getName(), globalActor,
-              /*actorIndependent=*/false, useKind,
-              result == AsyncMarkingResult::SyncContext);
-          }
-          noteIsolatedActorMember(value, context);
-        };
-
-        if (AbstractFunctionDecl const* fn =
-            dyn_cast_or_null<AbstractFunctionDecl>(declContext->getAsDecl())) {
-          bool isAsyncContext = fn->isAsyncContext();
-
-          if (implicitlyAsyncExpr && isAsyncContext)
-            return didEmitDiagnostic; // definitely an OK reference.
-
-          // otherwise, there's something wrong.
-          
-          // if it's an implicitly-async call in a non-async context,
-          // then we know later type-checking will raise an error,
-          // so we just emit a note pointing out that callee of the call is
-          // implicitly async.
-          emitError(/*justNote=*/implicitlyAsyncExpr);
-
-          // otherwise, if it's any kind of global-actor reference within
-          // this synchronous function, we'll additionally suggest becoming
-          // part of the global actor associated with the reference,
-          // since this function is not associated with an actor.
-          if (isa<FuncDecl>(fn) && !isAsyncContext) {
-            didEmitDiagnostic = true;
-            fn->diagnose(diag::note_add_globalactor_to_function, 
+        // Diagnose the reference.
+        auto useKind = static_cast<unsigned>(
+            kindOfUsage(value, context).getValueOr(VarRefUseEnv::Read));
+        ctx.Diags.diagnose(
+          loc, diag::global_actor_from_nonactor_context,
+          value->getDescriptiveKind(), value->getName(), globalActor,
+          /*actorIndependent=*/false, useKind,
+          result == AsyncMarkingResult::SyncContext);
+
+        // If we are in a synchronous function on the global actor,
+        // suggest annotating with the global actor itself.
+        if (auto fn = dyn_cast<FuncDecl>(declContext)) {
+          if (!isa<AccessorDecl>(fn) && !fn->isAsyncContext()) {
+            fn->diagnose(diag::note_add_globalactor_to_function,
                 globalActor->getWithoutParens().getString(),
                 fn->getDescriptiveKind(),
                 fn->getName(),
                 globalActor)
-              .fixItInsert(fn->getAttributeInsertionLoc(false), 
+              .fixItInsert(fn->getAttributeInsertionLoc(false),
                 diag::insert_globalactor_attr, globalActor);
           }
-
-        } else {
-          // just the generic error with note.
-          emitError();
         }
 
-        return didEmitDiagnostic;
+        noteIsolatedActorMember(value, context);
+
+        return true;
       } // end Unspecified case
       } // end switch
       llvm_unreachable("unhandled actor isolation kind!");

test/Concurrency/actor_isolation.swift

@@ -655,4 +655,15 @@ class SomeClassWithInits {
   }
 
   func isolated() { }
+
+  func hasDetached() {
+    Task.runDetached {
+      // okay
+      await self.isolated() // expected-warning{{cannot use parameter 'self' with a non-concurrent-value type 'SomeClassWithInits' from concurrently-executed code}}
+      self.isolated() // expected-warning{{cannot use parameter 'self' with a non-concurrent-value type 'SomeClassWithInits' from concurrently-executed code}}
+      // expected-error@-1{{call is 'async' but is not marked with 'await'}}
+
+      print(await self.mutableState) // expected-warning{{cannot use parameter 'self' with a non-concurrent-value type 'SomeClassWithInits' from concurrently-executed code}}
+    }
+  }
 }

test/Concurrency/actor_isolation_unsafe.swift

@@ -0,0 +1,78 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+
+@globalActor
+actor SomeGlobalActor {
+  static let shared = SomeGlobalActor()
+}
+
+@MainActor(unsafe) func globalMain() { } // expected-note {{calls to global function 'globalMain()' from outside of its actor context are implicitly asynchronous}}
+
+@SomeGlobalActor(unsafe) func globalSome() { } // expected-note 2{{calls to global function 'globalSome()' from outside of its actor context are implicitly asynchronous}}
+
+// ----------------------------------------------------------------------
+// Witnessing and unsafe global actor
+// ----------------------------------------------------------------------
+protocol P1 {
+  @MainActor(unsafe) func onMainActor() // expected-note{{'onMainActor()' declared here}}
+}
+
+struct S1_P1: P1 {
+  func onMainActor() { }
+}
+
+struct S2_P1: P1 {
+  @MainActor func onMainActor() { }
+}
+
+struct S3_P1: P1 {
+  @actorIndependent func onMainActor() { }
+}
+
+struct S4_P1: P1 {
+  @SomeGlobalActor func onMainActor() { } // expected-error{{instance method 'onMainActor()' isolated to global actor 'SomeGlobalActor' can not satisfy corresponding requirement from protocol 'P1' isolated to global actor 'MainActor'}}
+}
+
+// ----------------------------------------------------------------------
+// Overriding and unsafe global actor
+// ----------------------------------------------------------------------
+class C1 {
+  @MainActor(unsafe) func method() { } // expected-note{{overridden declaration is here}}
+}
+
+class C2: C1 {
+  override func method() { // expected-note{{overridden declaration is here}}
+    globalSome() // okay
+  }
+}
+
+class C3: C1 {
+  @actorIndependent override func method() {
+    globalSome() // expected-error{{global function 'globalSome()' isolated to global actor 'SomeGlobalActor' can not be referenced from an '@actorIndependent' synchronous context}}
+  }
+}
+
+class C4: C1 {
+  @MainActor override func method() {
+    globalSome() // expected-error{{global function 'globalSome()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'MainActor'}}
+  }
+}
+
+class C5: C1 {
+  @SomeGlobalActor override func method() { // expected-error{{global actor 'SomeGlobalActor'-isolated instance method 'method()' has different actor isolation from global actor 'MainActor'-isolated overridden declaration}}
+  }
+}
+
+class C6: C2 {
+  // We didn't infer any actor isolation for C2.method().
+  @SomeGlobalActor override func method() { // expected-error{{global actor 'SomeGlobalActor'-isolated instance method 'method()' has different actor isolation from non-actor-isolated overridden declaration}}
+  }
+}
+
+class C7: C2 {
+  // We didn't infer any actor isolation for C2.method(), but it's okay to be
+  // explicitly unsafe.
+  @SomeGlobalActor(unsafe) override func method() {
+    globalMain() // expected-error{{global function 'globalMain()' isolated to global actor 'MainActor' can not be referenced from different global actor 'SomeGlobalActor'}}
+    globalSome() // okay
+  }
+}

test/Concurrency/concurrency_warnings.swift

@@ -0,0 +1,19 @@
+// RUN: %target-typecheck-verify-swift -warn-concurrency
+// REQUIRES: concurrency
+
+class GlobalCounter {
+  var counter: Int = 0
+}
+
+let rs = GlobalCounter()
+var globalInt = 17 // expected-note 2{{var declared here}}
+
+class MyError: Error { // expected-warning{{non-final class 'MyError' cannot conform to `ConcurrentValue`; use `UnsafeConcurrentValue`}}
+  var storage = 0 // expected-warning{{stored property 'storage' of 'ConcurrentValue'-conforming class 'MyError' is mutable}}
+}
+
+func testWarnings() {
+  _ = rs // TODO: warn here
+  _ = globalInt // expected-warning{{reference to var 'globalInt' is not concurrency-safe because it involves shared mutable state}}
+  globalInt += 1 // expected-warning{{reference to var 'globalInt' is not concurrency-safe because it involves shared mutable state}}
+}