Merge pull request swiftlang#35048 from kavon/typechecking-unspecified-isolation-contexts-71548470

swift-ci · web-flow · commit 1eb810f56635 · 2020-12-14T17:43:15.000-08:00
diff --git a/include/swift/AST/DiagnosticsSema.def b/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))
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -113,8 +113,15 @@ 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 also emit a fix-it that inserts 'async' in the right place for func.
+    // It's possibly a bit tricky to get the right source location from
+    // just the AbstractFunctionDecl, but it's important to circle-back
+    // to this.
 
   if (func->canBeAsyncHandler()) {
     func->diagnose(
@@ -942,8 +949,9 @@ namespace {
         return false;
       };
 
+      auto declContext = getDeclContext();
       switch (auto contextIsolation =
-                  getInnermostIsolatedContext(getDeclContext())) {
+                  getInnermostIsolatedContext(declContext)) {
       case ActorIsolation::ActorInstance:
         if (inspectForImplicitlyAsync())
           return false;
@@ -980,16 +988,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!");
     }
 
diff --git a/lib/Sema/TypeCheckConcurrency.h b/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);
diff --git a/lib/Sema/TypeCheckEffects.cpp b/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) {
diff --git a/test/Concurrency/actor_call_implicitly_async.swift b/test/Concurrency/actor_call_implicitly_async.swift
@@ -125,8 +125,8 @@ func anotherAsyncFunc() async {
 
 }
 
-// expected-note@+2 {{add 'async' to function 'regularFunc()' to make it asynchronous}}
-// expected-note@+1 {{add '@asyncHandler' to function 'regularFunc()' to create an implicit asynchronous context}}
+// expected-note@+2 {{add 'async' to function 'regularFunc()' to make it asynchronous}} {{none}}
+// expected-note@+1 {{add '@asyncHandler' to function 'regularFunc()' to create an implicit asynchronous context}} {{1-1=@asyncHandler }}
 func regularFunc() {
   let a = BankAccount(initialDeposit: 34)
 
diff --git a/test/Concurrency/actor_isolation.swift b/test/Concurrency/actor_isolation.swift
@@ -44,7 +44,7 @@ extension MyActor {
     set { }
   }
 
-  // expected-note@+1 {{add 'async' to function 'actorIndependentFunc(otherActor:)' to make it asynchronous}}
+  // expected-note@+1 {{add 'async' to function 'actorIndependentFunc(otherActor:)' to make it asynchronous}} {{none}}
   @actorIndependent func actorIndependentFunc(otherActor: MyActor) -> Int {
     _ = immutable
     _ = text[0] // expected-error{{actor-isolated property 'text' can not be referenced from an '@actorIndependent' context}}
@@ -261,8 +261,8 @@ struct GenericStruct<T> {
     f() // okay
   }
 
-  // expected-note@+2 {{add '@asyncHandler' to function 'h()' to create an implicit asynchronous context}}
-  // expected-note@+1 {{add 'async' to function 'h()' to make it asynchronous}}
+  // expected-note@+2 {{add '@asyncHandler' to function 'h()' to create an implicit asynchronous context}} {{3-3=@asyncHandler }}
+  // expected-note@+1 {{add 'async' to function 'h()' to make it asynchronous}} {{none}}
   @GenericGlobalActor<String> func h() {
     f() // expected-error{{'async' in a function that does not support concurrency}}
     _ = f // expected-error{{instance method 'f()' isolated to global actor 'GenericGlobalActor<T>' can not be referenced from different global actor 'GenericGlobalActor<String>'}}
diff --git a/test/Concurrency/async_throwing.swift b/test/Concurrency/async_throwing.swift
@@ -44,8 +44,8 @@ func throwingTask() async throws -> String {
   return "ok!"
 }
 
-// expected-note@+2 7 {{add '@asyncHandler' to function 'syncTest()' to create an implicit asynchronous context}}
-// expected-note@+1 7 {{add 'async' to function 'syncTest()' to make it asynchronous}}
+// expected-note@+2 7 {{add '@asyncHandler' to function 'syncTest()' to create an implicit asynchronous context}} {{1-1=@asyncHandler }}
+// expected-note@+1 7 {{add 'async' to function 'syncTest()' to make it asynchronous}} {{none}}
 func syncTest() {
   let _ = invoke(fn: normalTask) // expected-error{{'async' in a function that does not support concurrency}}
   let _ = invokeAuto(42) // expected-error{{'async' in a function that does not support concurrency}}
diff --git a/test/Concurrency/global_actor_from_ordinary_context.swift b/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'}} {{1-1=@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'}} {{1-1=@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}} {{1-1=@asyncHandler }}
+// expected-note@+1 {{add 'async' to function 'referenceAsyncGlobalActor()' to make it asynchronous}} {{none}}
+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}} {{1-1=@asyncHandler }}
+// expected-note@+2 {{add 'async' to function 'callGlobalActor()' to make it asynchronous}} {{none}}
+// expected-note@+1 {{add '@SomeGlobalActor' to make global function 'callGlobalActor()' part of global actor 'SomeGlobalActor'}} {{1-1=@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}} {{none}}
+    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'}} {{3-3=@SomeGlobalActor }}
+  // expected-note@+2 {{add '@asyncHandler' to function 'method1()' to create an implicit asynchronous context}} {{3-3=@asyncHandler }}
+  // expected-note@+1 {{add 'async' to function 'method1()' to make it asynchronous}} {{none}}
+  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'}} {{3-3=@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}}
+}
diff --git a/test/Concurrency/global_actor_inference.swift b/test/Concurrency/global_actor_inference.swift
diff --git a/test/attr/asynchandler.swift b/test/attr/asynchandler.swift
diff --git a/test/attr/attr_objc_async.swift b/test/attr/attr_objc_async.swift
diff --git a/test/expr/unary/async_await.swift b/test/expr/unary/async_await.swift

Original file line number	Diff line number	Diff line change
`@@ -125,8 +125,8 @@ func anotherAsyncFunc() async {`
`125`	`125`
`126`	`126`	`}`
`127`	`127`
`128`		`-// expected-note@+2 {{add 'async' to function 'regularFunc()' to make it asynchronous}}`
`129`		`-// expected-note@+1 {{add '@asyncHandler' to function 'regularFunc()' to create an implicit asynchronous context}}`
	`128`	`+// expected-note@+2 {{add 'async' to function 'regularFunc()' to make it asynchronous}} {{none}}`
	`129`	`+// expected-note@+1 {{add '@asyncHandler' to function 'regularFunc()' to create an implicit asynchronous context}} {{1-1=@asyncHandler }}`
`130`	`130`	`func regularFunc() {`
`131`	`131`	`let a = BankAccount(initialDeposit: 34)`
`132`	`132`