Enforce safe access to unsafe global actor declarations only from "new" code.

Allow references to unsafe global actor-isolated declarations only from
existing code that has not adopted concurrency features (such as
async, @Concurrent closures, etc.). This allows declarations that
should be isolated to a global actor to be annotated as such without
breaking existing code (as if isolation was unspecified), while code
that does adopt concurrency will treat the declaration as being part
of that global actor.

Author: DougGregor

lib/Sema/TypeCheckConcurrency.cpp

@@ -554,23 +554,21 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
       return forActorSelf(isolation.getActor(),
           isAccessibleAcrossActors || isa<ConstructorDecl>(decl));
 
+    case ActorIsolation::GlobalActorUnsafe:
     case ActorIsolation::GlobalActor: {
       Type actorType = isolation.getGlobalActor();
       if (auto subs = declRef.getSubstitutions())
         actorType = actorType.subst(subs);
 
-      return forGlobalActor(actorType, isAccessibleAcrossActors);
+      return forGlobalActor(actorType, isAccessibleAcrossActors,
+                            isolation == ActorIsolation::GlobalActorUnsafe);
     }
 
     case ActorIsolation::Independent:
     case ActorIsolation::IndependentUnsafe:
       // Actor-independent have no restrictions on their access.
       return forUnrestricted();
 
-    case ActorIsolation::GlobalActorUnsafe:
-      // TODO: Capture GlobalActorUnsafe so we can do more checking.
-      return forUnrestricted();
-
     case ActorIsolation::Unspecified:
       return isAccessibleAcrossActors ? forUnrestricted() : forUnsafe();
     }
@@ -1297,6 +1295,14 @@ namespace {
         case ActorIsolationRestriction::Unrestricted:
         case ActorIsolationRestriction::Unsafe:
           break;
+        case ActorIsolationRestriction::GlobalActorUnsafe:
+          // If we're not supposed to diagnose existing data races here,
+          // we're done.
+          if (!shouldDiagnoseExistingDataRaces(getDeclContext()))
+            break;
+
+          LLVM_FALLTHROUGH;
+
         case ActorIsolationRestriction::GlobalActor: {
           ctx.Diags.diagnose(argLoc, diag::actor_isolated_inout_state,
                              decl->getDescriptiveKind(), decl->getName(),
@@ -1682,6 +1688,13 @@ namespace {
       case ActorIsolationRestriction::ActorSelf:
         llvm_unreachable("non-member reference into an actor");
 
+      case ActorIsolationRestriction::GlobalActorUnsafe:
+        // Only complain if we're in code that's adopted concurrency features.
+        if (!shouldDiagnoseExistingDataRaces(getDeclContext()))
+          return false;
+
+        LLVM_FALLTHROUGH;
+
       case ActorIsolationRestriction::GlobalActor:
         return checkGlobalActorReference(
             valueRef, loc, isolation.getGlobalActor(), isolation.isCrossActor);
@@ -1852,6 +1865,13 @@ namespace {
         llvm_unreachable("Unhandled actor isolation");
       }
 
+      case ActorIsolationRestriction::GlobalActorUnsafe:
+        // Only complain if we're in code that's adopted concurrency features.
+        if (!shouldDiagnoseExistingDataRaces(getDeclContext()))
+          return false;
+
+        LLVM_FALLTHROUGH;
+
       case ActorIsolationRestriction::GlobalActor:
         return checkGlobalActorReference(
             memberRef, memberLoc, isolation.getGlobalActor(),
@@ -2190,13 +2210,14 @@ ActorIsolation ActorIsolationRequest::evaluate(
                               ActorIndependentKind::Safe, /*IsImplicit=*/true));
       break;
 
-    case ActorIsolation::GlobalActor:
-    case ActorIsolation::GlobalActorUnsafe: {
+    case ActorIsolation::GlobalActorUnsafe:
+      // Don't infer unsafe global actor isolation.
+      return ActorIsolation::forUnspecified();
+
+    case ActorIsolation::GlobalActor: {
       auto typeExpr = TypeExpr::createImplicit(inferred.getGlobalActor(), ctx);
       auto attr = CustomAttr::create(
           ctx, SourceLoc(), typeExpr, /*implicit=*/true);
-      if (inferred == ActorIsolation::GlobalActorUnsafe)
-        attr->setArgIsUnsafe(true);
       value->getAttrs().add(attr);
       break;
     }
@@ -2354,6 +2375,31 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
     }
   }
 
+  // If the overriding declaration uses an unsafe global actor, we can do
+  // anything that doesn't actively conflict with the overridden isolation.
+  if (isolation == ActorIsolation::GlobalActorUnsafe) {
+    switch (overriddenIsolation) {
+    case ActorIsolation::Unspecified:
+      return;
+
+    case ActorIsolation::ActorInstance:
+    case ActorIsolation::Independent:
+    case ActorIsolation::IndependentUnsafe:
+      // Diagnose below.
+      break;
+
+    case ActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActorUnsafe:
+      // The global actors don't match; diagnose it.
+      if (overriddenIsolation.getGlobalActor()->isEqual(
+              isolation.getGlobalActor()))
+        return;
+
+      // Diagnose below.
+      break;
+    }
+  }
+
   // Isolation mismatch. Diagnose it.
   value->diagnose(
       diag::actor_isolation_override_mismatch, isolation,

lib/Sema/TypeCheckConcurrency.h

@@ -90,6 +90,12 @@ class ActorIsolationRestriction {
     /// permitted from other declarations with that same global actor or
     /// are permitted from elsewhere as a cross-actor reference.
     GlobalActor,
+
+    /// References to a declaration that is part of a global actor are
+    /// permitted from other declarations with that same global actor or
+    /// are permitted from elsewhere as a cross-actor reference, but
+    /// contexts with unspecified isolation won't diagnose anything.
+    GlobalActorUnsafe,
   };
 
 private:
@@ -125,7 +131,7 @@ class ActorIsolationRestriction {
 
   /// Retrieve the actor class that the declaration is within.
   Type getGlobalActor() const {
-    assert(kind == GlobalActor);
+    assert(kind == GlobalActor || kind == GlobalActorUnsafe);
     return Type(data.globalActor);
   }
 
@@ -152,8 +158,9 @@ class ActorIsolationRestriction {
   /// Accesses to the given declaration can only be made via this particular
   /// global actor or is a cross-actor access.
   static ActorIsolationRestriction forGlobalActor(
-      Type globalActor, bool isCrossActor) {
-    ActorIsolationRestriction result(GlobalActor, isCrossActor);
+      Type globalActor, bool isCrossActor, bool isUnsafe) {
+    ActorIsolationRestriction result(
+        isUnsafe ? GlobalActorUnsafe : GlobalActor, isCrossActor);
     result.data.globalActor = globalActor.getPointer();
     return result;
   }

lib/Sema/TypeCheckDeclObjC.cpp

@@ -408,6 +408,7 @@ static bool checkObjCActorIsolation(const ValueDecl *VD,
     }
     return true;
 
+  case ActorIsolationRestriction::GlobalActorUnsafe:
   case ActorIsolationRestriction::GlobalActor:
     // FIXME: Consider whether to limit @objc on global-actor-qualified
     // declarations.

lib/Sema/TypeCheckProtocol.cpp

@@ -2725,6 +2725,7 @@ bool ConformanceChecker::checkActorIsolation(
   // Ensure that the witness is not actor-isolated in a manner that makes it
   // unsuitable as a witness.
   bool isCrossActor = false;
+  bool witnessIsUnsafe = false;
   Type witnessGlobalActor;
   switch (auto witnessRestriction =
               ActorIsolationRestriction::forDeclaration(witness)) {
@@ -2762,6 +2763,10 @@ bool ConformanceChecker::checkActorIsolation(
     return diagnoseNonConcurrentTypesInReference(
         witness, DC, witness->getLoc(), ConcurrentReferenceKind::CrossActor);
 
+  case ActorIsolationRestriction::GlobalActorUnsafe:
+    witnessIsUnsafe = true;
+    LLVM_FALLTHROUGH;
+
   case ActorIsolationRestriction::GlobalActor: {
     // Hang on to the global actor that's used for the witness. It will need
     // to match that of the requirement.
@@ -2839,6 +2844,10 @@ bool ConformanceChecker::checkActorIsolation(
     if (requirement->hasClangNode())
       return false;
 
+    // If the witness is "unsafe", allow the mismatch.
+    if (witnessIsUnsafe)
+      return false;
+
     witness->diagnose(
         diag::global_actor_isolated_witness, witness->getDescriptiveKind(),
         witness->getName(), witnessGlobalActor, Proto->getName());

test/Concurrency/global_actor_inference.swift

@@ -239,7 +239,7 @@ func barSync() {
 // Unsafe global actors
 // ----------------------------------------------------------------------
 protocol UGA {
-  @SomeGlobalActor(unsafe) func req()
+  @SomeGlobalActor(unsafe) func req() // expected-note{{calls to instance method 'req()' from outside of its actor context are implicitly asynchronous}}
 }
 
 struct StructUGA1: UGA {
@@ -252,7 +252,7 @@ struct StructUGA2: UGA {
 
 @GenericGlobalActor<String>
 func testUGA<T: UGA>(_ value: T) {
-  value.req()
+  value.req() // expected-error{{instance method 'req()' isolated to global actor 'SomeGlobalActor' can not be referenced from different global actor 'GenericGlobalActor<String>'}}
 }
 
 class UGAClass {