[Global actors] Check calls to global-actor-qualified functions, not references

Check actor isolation of calls to functions with global-actor-qualified
type. This closes a pre-existing loophole where a value of
global-actor-qualified function type could be called from any context.
Paired with this, references to global-actor-qualified function
declarations will get global-actor-qualified function type whenever
they are referenced within an experience, i.e., whenever we form a
value of that type. Such references can occur anywhere (one does not
need to be on the actor), and carrying the global actor along with the
function type ensures that they can only be called from the right
actor. For example:

    @mainactor func onlyOnMainActor() { ... }

    func callIt(_ fn: @mainactor () -> Void) {
      fn() // error: not on the main actor, so cannot synchronously call
           // this wasn't previously diagnosed
    }

    func passIt() {
      callIt(onlyOnMainActor)  // okay to pass the function
                               // used to be an error
    }

While here, fix up some broken substitution logic for
global-actor-qualified function types and "override" actor isolation.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4385,6 +4385,12 @@ ERROR(global_actor_from_nonactor_context,none,
       "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
       " from %select{this|a non-isolated}3%select{| synchronous}5 context",
       (DescriptiveDeclKind, DeclName, Type, bool, unsigned, bool))
+ERROR(actor_isolated_call,none,
+      "call to %0 function in a synchronous %1 context",
+      (ActorIsolation, ActorIsolation))
+ERROR(actor_isolated_call_decl,none,
+      "call to %0 %1 %2 in a synchronous %3 context",
+      (ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
 ERROR(actor_isolated_partial_apply,none,
         "actor-isolated %0 %1 can not be partially applied",
         (DescriptiveDeclKind, DeclName))

lib/AST/ASTContext.cpp

@@ -3075,11 +3075,13 @@ DynamicSelfType *DynamicSelfType::get(Type selfType, const ASTContext &ctx) {
 
 static RecursiveTypeProperties
 getFunctionRecursiveProperties(ArrayRef<AnyFunctionType::Param> params,
-                               Type result) {
+                               Type result, Type globalActor) {
   RecursiveTypeProperties properties;
   for (auto param : params)
     properties |= param.getPlainType()->getRecursiveProperties();
   properties |= result->getRecursiveProperties();
+  if (globalActor)
+    properties |= globalActor->getRecursiveProperties();
   properties &= ~RecursiveTypeProperties::IsLValue;
   return properties;
 }
@@ -3274,7 +3276,11 @@ void FunctionType::Profile(llvm::FoldingSetNodeID &ID,
 
 FunctionType *FunctionType::get(ArrayRef<AnyFunctionType::Param> params,
                                 Type result, Optional<ExtInfo> info) {
-  auto properties = getFunctionRecursiveProperties(params, result);
+  Type globalActor;
+  if (info.hasValue())
+    globalActor = info->getGlobalActor();
+
+  auto properties = getFunctionRecursiveProperties(params, result, globalActor);
   auto arena = getArena(properties);
 
   llvm::FoldingSetNodeID id;
@@ -3296,10 +3302,6 @@ FunctionType *FunctionType::get(ArrayRef<AnyFunctionType::Param> params,
   bool hasClangInfo =
       info.hasValue() && !info.getValue().getClangTypeInfo().empty();
 
-  Type globalActor;
-  if (info.hasValue())
-    globalActor = info->getGlobalActor();
-
   size_t allocSize = totalSizeToAlloc<
       AnyFunctionType::Param, ClangTypeInfo, Type
     >(params.size(), hasClangInfo ? 1 : 0, globalActor ? 1 : 0);
@@ -3393,7 +3395,7 @@ GenericFunctionType *GenericFunctionType::get(GenericSignature sig,
   if (info.hasValue())
     globalActor = info->getGlobalActor();
 
-  if (globalActor && !globalActor->isCanonical())
+  if (globalActor && !sig->isCanonicalTypeInContext(globalActor))
     isCanonical = false;
 
   size_t allocSize = totalSizeToAlloc<AnyFunctionType::Param, Type>(

lib/AST/DiagnosticEngine.cpp

@@ -475,6 +475,18 @@ static Type getAkaTypeForDisplay(Type type) {
   });
 }
 
+/// Determine whether this is the main actor type.
+static bool isMainActor(Type type) {
+  if (auto nominal = type->getAnyNominal()) {
+    if (nominal->getName().is("MainActor") &&
+        nominal->getParentModule()->getName() ==
+          nominal->getASTContext().Id_Concurrency)
+      return true;
+  }
+
+  return false;
+}
+
 /// Format a single diagnostic argument and write it to the given
 /// stream.
 static void formatDiagnosticArgument(StringRef Modifier, 
@@ -710,18 +722,21 @@ static void formatDiagnosticArgument(StringRef Modifier,
       break;
 
     case ActorIsolation::GlobalActor:
-    case ActorIsolation::GlobalActorUnsafe:
-      Out << "global actor " << FormatOpts.OpeningQuotationMark
-        << isolation.getGlobalActor().getString()
-        << FormatOpts.ClosingQuotationMark << "-isolated";
+    case ActorIsolation::GlobalActorUnsafe: {
+      Type globalActor = isolation.getGlobalActor();
+      if (isMainActor(globalActor)) {
+        Out << "main actor-isolated";
+      } else {
+        Out << "global actor " << FormatOpts.OpeningQuotationMark
+          << globalActor.getString()
+          << FormatOpts.ClosingQuotationMark << "-isolated";
+      }
       break;
+    }
 
     case ActorIsolation::Independent:
-      Out << "actor-independent";
-      break;
-
     case ActorIsolation::Unspecified:
-      Out << "unspecified";
+      Out << "nonisolated";
       break;
     }
   }

lib/AST/Type.cpp

@@ -4879,6 +4879,17 @@ case TypeKind::Id:
     if (resultTy.getPointer() != function->getResult().getPointer())
       isUnchanged = false;
 
+    // Transform the global actor.
+    Type globalActorType;
+    if (Type origGlobalActorType = function->getGlobalActor()) {
+      globalActorType = origGlobalActorType.transformRec(fn);
+      if (!globalActorType)
+        return Type();
+
+      if (globalActorType.getPointer() != origGlobalActorType.getPointer())
+        isUnchanged = false;
+    }
+
     if (auto genericFnType = dyn_cast<GenericFunctionType>(base)) {
 #ifndef NDEBUG
       // Check that generic parameters won't be trasnformed.
@@ -4895,15 +4906,17 @@ case TypeKind::Id:
       if (!function->hasExtInfo())
         return GenericFunctionType::get(genericSig, substParams, resultTy);
       return GenericFunctionType::get(genericSig, substParams, resultTy,
-                                      function->getExtInfo());
+                                      function->getExtInfo()
+                                          .withGlobalActor(globalActorType));
     }
 
     if (isUnchanged) return *this;
 
     if (!function->hasExtInfo())
       return FunctionType::get(substParams, resultTy);
     return FunctionType::get(substParams, resultTy,
-                             function->getExtInfo());
+                             function->getExtInfo()
+                                 .withGlobalActor(globalActorType));
   }
 
   case TypeKind::ArraySlice: {

lib/Sema/TypeCheckConcurrency.cpp

@@ -587,7 +587,7 @@ Type swift::getExplicitGlobalActor(ClosureExpr *closure) {
 
 /// Determine the isolation rules for a given declaration.
 ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
-    ConcreteDeclRef declRef) {
+    ConcreteDeclRef declRef, bool fromExpression) {
   auto decl = declRef.getDecl();
 
   switch (decl->getKind()) {
@@ -673,6 +673,13 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
 
     case ActorIsolation::GlobalActorUnsafe:
     case ActorIsolation::GlobalActor: {
+      // A global-actor-isolated function referenced within an expression
+      // carries the global actor into its function type. The actual
+      // reference to the function is therefore not restricted, because the
+      // call to the function is.
+      if (fromExpression && isa<AbstractFunctionDecl>(decl))
+        return forUnrestricted();
+
       Type actorType = isolation.getGlobalActor();
       if (auto subs = declRef.getSubstitutions())
         actorType = actorType.subst(subs);
@@ -1276,6 +1283,9 @@ namespace {
       if (auto apply = dyn_cast<ApplyExpr>(expr)) {
         applyStack.push_back(apply);  // record this encounter
 
+        // Check the call itself.
+        (void)checkApply(apply);
+
         // If this is a call to a partial apply thunk, decompose it to check it
         // like based on the original written syntax, e.g., "self.method".
         if (auto partialApply = decomposePartialApplyThunk(
@@ -1411,6 +1421,33 @@ namespace {
       return nullptr;
     }
 
+    /// Note when the enclosing context could be put on a global actor.
+    void noteGlobalActorOnContext(DeclContext *dc, Type globalActor) {
+      // If we are in a synchronous function on the global actor,
+      // suggest annotating with the global actor itself.
+      if (auto fn = dyn_cast<FuncDecl>(dc)) {
+        if (!isa<AccessorDecl>(fn) && !fn->isAsyncContext()) {
+          switch (getActorIsolation(fn)) {
+          case ActorIsolation::ActorInstance:
+          case ActorIsolation::GlobalActor:
+          case ActorIsolation::GlobalActorUnsafe:
+          case ActorIsolation::Independent:
+            return;
+
+          case ActorIsolation::Unspecified:
+            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);
+              return;
+          }
+        }
+      }
+    }
+
     /// Note that the given actor member is isolated.
     /// @param context is allowed to be null if no context is appropriate.
     void noteIsolatedActorMember(ValueDecl *decl, Expr *context) {
@@ -1672,6 +1709,86 @@ namespace {
       return result;
     }
 
+    /// Check actor isolation for a particular application.
+    bool checkApply(ApplyExpr *apply) {
+      auto fnExprType = apply->getFn()->getType();
+      if (!fnExprType)
+        return false;
+
+      auto fnType = fnExprType->getAs<FunctionType>();
+      if (!fnType)
+        return false;
+
+      // Handle calls to global-actor-qualified functions.
+      Type globalActor = fnType->getGlobalActor();
+      if (!globalActor)
+        return false;
+
+      auto declContext = const_cast<DeclContext *>(getDeclContext());
+
+      // Check whether we are within the same isolation context, in which
+      // case there is nothing further to check,
+      auto contextIsolation = getInnermostIsolatedContext(declContext);
+      if (contextIsolation.isGlobalActor() &&
+          contextIsolation.getGlobalActor()->isEqual(globalActor)) {
+        return false;
+      }
+
+      // From this point on, the only possibility is that we have an implicitly
+      // aynchronous call.
+
+      // If we are not in an asynchronous context, complain.
+      if (!isInAsynchronousContext()) {
+        auto isolation = ActorIsolation::forGlobalActor(
+            globalActor, /*unsafe=*/false);
+        if (auto calleeDecl = apply->getCalledValue()) {
+          ctx.Diags.diagnose(
+              apply->getLoc(), diag::actor_isolated_call_decl, isolation,
+              calleeDecl->getDescriptiveKind(), calleeDecl->getName(),
+              contextIsolation);
+          calleeDecl->diagnose(
+              diag::actor_isolated_sync_func, calleeDecl->getDescriptiveKind(),
+              calleeDecl->getName());
+        } else {
+          ctx.Diags.diagnose(
+              apply->getLoc(), diag::actor_isolated_call, isolation,
+              contextIsolation);
+        }
+        noteGlobalActorOnContext(
+            const_cast<DeclContext *>(getDeclContext()), globalActor);
+
+        return true;
+      }
+
+      // Mark as implicitly async.
+      apply->setImplicitlyAsync(true);
+
+      // If we don't need to check for sendability, we're done.
+      if (!shouldDiagnoseNonSendableViolations(ctx.LangOpts))
+        return false;
+
+      // Check for sendability of the parameter types.
+      for (const auto &param : fnType->getParams()) {
+        // FIXME: Dig out the locations of the corresponding arguments.
+        if (!isSendableType(getDeclContext(), param.getParameterType())) {
+          ctx.Diags.diagnose(
+              apply->getLoc(), diag::non_concurrent_param_type,
+              param.getParameterType());
+          return true;
+        }
+      }
+
+      // Check for sendability of the result type.
+      if (!isSendableType(getDeclContext(), fnType->getResult())) {
+        ctx.Diags.diagnose(
+            apply->getLoc(), diag::non_concurrent_result_type,
+            fnType->getResult());
+        return true;
+      }
+
+      return false;
+    }
+
     /// Check a reference to an entity within a global actor.
     bool checkGlobalActorReference(
         ConcreteDeclRef valueRef, SourceLoc loc, Type globalActor,
@@ -1763,21 +1880,7 @@ namespace {
           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),
-                diag::insert_globalactor_attr, globalActor);
-          }
-        }
-
+        noteGlobalActorOnContext(declContext, globalActor);
         noteIsolatedActorMember(value, context);
 
         return true;
@@ -2733,10 +2836,10 @@ ActorIsolation ActorIsolationRequest::evaluate(
     if (!isAsyncHandler(value)) {
       auto isolation = getActorIsolation(overriddenValue);
       SubstitutionMap subs;
-      if (auto env = value->getInnermostDeclContext()
-              ->getGenericEnvironmentOfContext()) {
-        subs = SubstitutionMap::getOverrideSubstitutions(
-          overriddenValue, value, subs);
+
+      if (Type selfType = value->getDeclContext()->getSelfInterfaceType()) {
+        subs = selfType->getMemberSubstitutionMap(
+            value->getModuleContext(), overriddenValue);
       }
 
       return inferredIsolation(isolation.subst(subs));
@@ -2881,11 +2984,12 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
 
   if (overriddenIsolation.requiresSubstitution()) {
     SubstitutionMap subs;
-    if (auto env = value->getInnermostDeclContext()
-            ->getGenericEnvironmentOfContext()) {
-      subs = SubstitutionMap::getOverrideSubstitutions(overridden, value, subs);
-      overriddenIsolation = overriddenIsolation.subst(subs);
+    if (Type selfType = value->getDeclContext()->getSelfInterfaceType()) {
+      subs = selfType->getMemberSubstitutionMap(
+          value->getModuleContext(), overridden);
     }
+
+    overriddenIsolation = overriddenIsolation.subst(subs);
   }
 
   // If the isolation matches, we're done.

lib/Sema/TypeCheckConcurrency.h

@@ -169,7 +169,11 @@ class ActorIsolationRestriction {
   }
 
   /// Determine the isolation rules for a given declaration.
-  static ActorIsolationRestriction forDeclaration(ConcreteDeclRef declRef);
+  ///
+  /// \param fromExpression Indicates that the reference is coming from an
+  /// expression.
+  static ActorIsolationRestriction forDeclaration(
+      ConcreteDeclRef declRef, bool fromExpression = true);
 
   operator Kind() const { return kind; };
 };

lib/Sema/TypeCheckDeclObjC.cpp

@@ -403,7 +403,7 @@ static bool checkObjCActorIsolation(const ValueDecl *VD,
   auto behavior = behaviorLimitForObjCReason(Reason, VD->getASTContext());
 
   switch (auto restriction = ActorIsolationRestriction::forDeclaration(
-              const_cast<ValueDecl *>(VD))) {
+              const_cast<ValueDecl *>(VD), /*fromExpression=*/false)) {
   case ActorIsolationRestriction::CrossActorSelf:
     // FIXME: Substitution map?
     diagnoseNonConcurrentTypesInReference(

lib/Sema/TypeCheckProtocol.cpp

@@ -2751,7 +2751,8 @@ bool ConformanceChecker::checkActorIsolation(
   bool witnessIsUnsafe = false;
   Type witnessGlobalActor;
   switch (auto witnessRestriction =
-              ActorIsolationRestriction::forDeclaration(witness)) {
+              ActorIsolationRestriction::forDeclaration(
+                  witness, /*fromExpression=*/false)) {
   case ActorIsolationRestriction::ActorSelf: {
     // An actor-isolated witness can only conform to an actor-isolated
     // requirement.