[concurrency] patch hole in typechecking ordinary functions for global actor references

Non-actor isolated synchronous functions were previously
allowed to call & reference global-actor isolated declarations.
This patch puts a stop to that.

Resolves rdar://71548470

Author: kavon

include/swift/AST/DiagnosticsSema.def

@@ -4132,6 +4132,10 @@ NOTE(note_add_async_to_function,none,
      "add 'async' to function %0 to make it asynchronous", (DeclName))
 NOTE(note_add_asynchandler_to_function,none,
      "add '@asyncHandler' to function %0 to create an implicit asynchronous context", (DeclName))
+NOTE(note_add_globalactor_to_function,none,
+     "add '@%0' to make %1 %2 part of global actor %3", 
+     (StringRef, DescriptiveDeclKind, DeclName, Type))
+FIXIT(insert_globalactor_attr, "@%0 ", (Type))
 ERROR(not_objc_function_async,none,
       "'async' function cannot be represented in Objective-C", ())
 NOTE(not_objc_function_type_async,none,
@@ -4217,10 +4221,10 @@ ERROR(global_actor_from_other_global_actor_context,none,
       "%0 %1 isolated to global actor %2 can not be referenced from "
       "different global actor %3",
       (DescriptiveDeclKind, DeclName, Type, Type))
-ERROR(global_actor_from_independent_context,none,
-      "%0 %1 isolated to global actor %2 can not be referenced from an "
-      "'@actorIndependent' context",
-      (DescriptiveDeclKind, DeclName, Type))
+ERROR(global_actor_from_nonactor_context,none,
+      "%0 %1 isolated to global actor %2 can not be referenced from "
+      "%select{this|an '@actorIndependent'}3 context",
+      (DescriptiveDeclKind, DeclName, Type, bool))
 ERROR(actor_isolated_partial_apply,none,
         "actor-isolated %0 %1 can not be partially applied",
         (DescriptiveDeclKind, DeclName))

lib/Sema/TypeCheckConcurrency.cpp

@@ -113,8 +113,12 @@ static bool checkAsyncHandler(FuncDecl *func, bool diagnose) {
   return false;
 }
 
-void swift::addAsyncNotes(FuncDecl *func) {
-  func->diagnose(diag::note_add_async_to_function, func->getName());
+void swift::addAsyncNotes(AbstractFunctionDecl const* func) {
+  assert(func);
+  if (!isa<DestructorDecl>(func))
+    func->diagnose(diag::note_add_async_to_function, func->getName());
+    // TODO: we need a source location for effects attributes so that we 
+    // can emit a fix-it.
 
   if (func->canBeAsyncHandler()) {
     func->diagnose(
@@ -942,8 +946,9 @@ namespace {
         return false;
       };
 
+      auto declContext = getDeclContext();
       switch (auto contextIsolation =
-                  getInnermostIsolatedContext(getDeclContext())) {
+                  getInnermostIsolatedContext(declContext)) {
       case ActorIsolation::ActorInstance:
         if (inspectForImplicitlyAsync())
           return false;
@@ -980,16 +985,66 @@ namespace {
           return false;
 
         ctx.Diags.diagnose(
-            loc, diag::global_actor_from_independent_context,
-            value->getDescriptiveKind(), value->getName(), globalActor);
+            loc, diag::global_actor_from_nonactor_context,
+            value->getDescriptiveKind(), value->getName(), globalActor,
+            /*actorIndependent=*/true);
         noteIsolatedActorMember(value);
         return true;
 
-      case ActorIsolation::Unspecified:
-        // Okay no matter what, but still must inspect for implicitly async.
-        inspectForImplicitlyAsync();
-        return false;
-      }
+      case ActorIsolation::Unspecified: {
+        // NOTE: we must always inspect for implicitlyAsync
+        bool implicitlyAsyncCall = inspectForImplicitlyAsync();
+        bool didEmitDiagnostic = false;
+
+        auto emitError = [&](bool justNote = false) {
+          didEmitDiagnostic = true;
+          if (!justNote) {
+            ctx.Diags.diagnose(
+              loc, diag::global_actor_from_nonactor_context,
+              value->getDescriptiveKind(), value->getName(), globalActor,
+              /*actorIndependent=*/false);
+          }
+          noteIsolatedActorMember(value);
+        };
+        
+        if (AbstractFunctionDecl const* fn = 
+            dyn_cast_or_null<AbstractFunctionDecl>(declContext->getAsDecl())) {
+          bool isAsyncContext = fn->isAsyncContext();
+
+          if (implicitlyAsyncCall && 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=*/implicitlyAsyncCall);
+
+          // 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, 
+                globalActor->getWithoutParens().getString(),
+                fn->getDescriptiveKind(),
+                fn->getName(),
+                globalActor)
+              .fixItInsert(fn->getAttributeInsertionLoc(false), 
+                diag::insert_globalactor_attr, globalActor);
+          }
+
+        } else {
+          // just the generic error with note.
+          emitError();
+        }
+
+        return didEmitDiagnostic;
+      } // end Unspecified case
+      } // end switch
       llvm_unreachable("unhandled actor isolation kind!");
     }
 

lib/Sema/TypeCheckConcurrency.h

@@ -40,7 +40,7 @@ class ValueDecl;
 
 /// Add notes suggesting the addition of 'async' or '@asyncHandler', as
 /// appropriate, to a diagnostic for a function that isn't an async context.
-void addAsyncNotes(FuncDecl *func);
+void addAsyncNotes(AbstractFunctionDecl const* func);
 
 /// Check actor isolation rules.
 void checkTopLevelActorIsolation(TopLevelCodeDecl *decl);

lib/Sema/TypeCheckEffects.cpp

@@ -1382,11 +1382,8 @@ class Context {
     if (!Function)
       return;
 
-    auto func = dyn_cast_or_null<FuncDecl>(Function->getAbstractFunctionDecl());
-    if (!func)
-      return;
-
-    addAsyncNotes(func);
+    if (auto func = Function->getAbstractFunctionDecl())
+      addAsyncNotes(func);
   }
 
   void diagnoseUnhandledAsyncSite(DiagnosticEngine &Diags, ASTNode node) {

test/Concurrency/global_actor_from_ordinary_context.swift

@@ -0,0 +1,131 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+// REQUIRES: concurrency
+
+// provides coverage for rdar://71548470
+
+actor class TestActor {}
+
+@globalActor
+struct SomeGlobalActor {
+  static var shared: TestActor { TestActor() }
+}
+
+// expected-note@+1 13 {{calls to global function 'syncGlobActorFn()' from outside of its actor context are implicitly asynchronous}}
+@SomeGlobalActor func syncGlobActorFn() { }
+@SomeGlobalActor func asyncGlobalActFn() async { }
+
+actor class Alex {
+  @SomeGlobalActor let const_memb = 20
+  @SomeGlobalActor var mut_memb = 30 // expected-note 2 {{mutable state is only available within the actor instance}}
+  @SomeGlobalActor func method() {} // expected-note 2 {{calls to instance method 'method()' from outside of its actor context are implicitly asynchronous}}
+  @SomeGlobalActor subscript(index : Int) -> Int { // expected-note 4 {{subscript declared here}}
+    get {
+      return index * 2
+    }
+    set {}
+  }
+}
+
+
+// expected-note@+1 4 {{add '@SomeGlobalActor' to make global function 'referenceGlobalActor()' part of global actor 'SomeGlobalActor'}}
+func referenceGlobalActor() {
+  let a = Alex()
+  // expected-error@+1 {{instance method 'method()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  _ = a.method
+  _ = a.const_memb
+  _ = a.mut_memb  // expected-error{{property 'mut_memb' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+
+  _ = a[1]  // expected-error{{subscript 'subscript(_:)' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  a[0] = 1  // expected-error{{subscript 'subscript(_:)' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+}
+
+
+// expected-note@+1 {{add '@SomeGlobalActor' to make global function 'referenceGlobalActor2()' part of global actor 'SomeGlobalActor'}}
+func referenceGlobalActor2() {
+  // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  let x = syncGlobActorFn
+  x()
+}
+
+
+// expected-note@+2 {{add '@asyncHandler' to function 'referenceAsyncGlobalActor()' to create an implicit asynchronous context}}
+// expected-note@+1 {{add 'async' to function 'referenceAsyncGlobalActor()' to make it asynchronous}}
+func referenceAsyncGlobalActor() {
+  let y = asyncGlobalActFn
+  y() // expected-error{{'async' in a function that does not support concurrency}}
+}
+
+
+// expected-note@+3 {{add '@asyncHandler' to function 'callGlobalActor()' to create an implicit asynchronous context}}
+// expected-note@+2 {{add 'async' to function 'callGlobalActor()' to make it asynchronous}}
+// expected-note@+1 {{add '@SomeGlobalActor' to make global function 'callGlobalActor()' part of global actor 'SomeGlobalActor'}}
+func callGlobalActor() {
+  syncGlobActorFn() // expected-error {{'async' in a function that does not support concurrency}}
+}
+
+func fromClosure() {
+  { () -> Void in
+    // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+    let x = syncGlobActorFn
+    x()
+  }()
+
+  // expected-error@+2 {{'async' in a function that does not support concurrency}}
+  // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  let _ = { syncGlobActorFn() }()
+}
+
+class Taylor {
+  init() { // expected-note {{add 'async' to function 'init()' to make it asynchronous}}
+    syncGlobActorFn() // expected-error {{'async' in a function that does not support concurrency}}
+
+    // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+    _ = syncGlobActorFn
+  }
+
+  deinit {
+    syncGlobActorFn() // expected-error {{'async' in a function that does not support concurrency}}
+
+    // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+    _ = syncGlobActorFn
+  }
+
+  // expected-note@+3 2 {{add '@SomeGlobalActor' to make instance method 'method1()' part of global actor 'SomeGlobalActor'}}
+  // expected-note@+2 {{add '@asyncHandler' to function 'method1()' to create an implicit asynchronous context}}
+  // expected-note@+1 {{add 'async' to function 'method1()' to make it asynchronous}}
+  func method1() {
+    syncGlobActorFn() // expected-error {{'async' in a function that does not support concurrency}}
+
+    // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+    _ = syncGlobActorFn
+  }
+
+  // expected-note@+2 2 {{add '@SomeGlobalActor' to make instance method 'cannotBeHandler()' part of global actor 'SomeGlobalActor'}}
+  // expected-note@+1 {{add 'async' to function 'cannotBeHandler()' to make it asynchronous}}
+  func cannotBeHandler() -> Int {
+    syncGlobActorFn() // expected-error {{'async' in a function that does not support concurrency}}
+
+    // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+    _ = syncGlobActorFn
+    return 0
+  }
+}
+
+
+func fromAsync() async {
+  // expected-error@+1 {{global function 'syncGlobActorFn()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  let x = syncGlobActorFn
+  x()
+
+  let y = asyncGlobalActFn
+  y() // expected-error{{call is 'async' but is not marked with 'await'}}
+
+  let a = Alex()
+  // expected-error@+1 {{instance method 'method()' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  _ = a.method
+  _ = a.const_memb
+  _ = a.mut_memb  // expected-error{{property 'mut_memb' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+
+  _ = a[1]  // expected-error{{subscript 'subscript(_:)' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+  a[0] = 1  // expected-error{{subscript 'subscript(_:)' isolated to global actor 'SomeGlobalActor' can not be referenced from this context}}
+}

test/Concurrency/global_actor_inference.swift

@@ -181,6 +181,7 @@ struct OtherContainer<U> {
 // Global actor inference for unspecified contexts
 // ----------------------------------------------------------------------
 
+// expected-note@+1 {{calls to global function 'foo()' from outside of its actor context are implicitly asynchronous}}
 @SomeGlobalActor func foo() { sibling() }
 
 @SomeGlobalActor func sibling() { foo() }
@@ -189,8 +190,9 @@ func bar() async {
   foo() // expected-error{{call is 'async' but is not marked with 'await'}}
 }
 
+// expected-note@+3 {{add '@SomeGlobalActor' to make global function 'barSync()' part of global actor 'SomeGlobalActor'}}
 // expected-note@+2 {{add '@asyncHandler' to function 'barSync()' to create an implicit asynchronous context}}
 // expected-note@+1 {{add 'async' to function 'barSync()' to make it asynchronous}}
 func barSync() {
-  foo() // expected-error{{'async' in a function that does not support concurrency}}
+  foo() // expected-error {{'async' in a function that does not support concurrency}}
 }