Merge pull request #36328 from kavon/actor-effectful-properties-pt2

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -4298,20 +4298,21 @@ ERROR(actor_isolated_mutating_func,none,
       "cannot call mutating async function %0 on actor-isolated %1 %2",
       (DeclName, DescriptiveDeclKind, DeclName))
 ERROR(actor_isolated_global_actor_context,none,
-      "actor-isolated %0 %1 can not be referenced from context of global "
-      "actor %2",
-      (DescriptiveDeclKind, DeclName, Type))
+      "actor-isolated %0 %1 can not be %select{referenced|mutated|used 'inout'}3 "
+      "from%select{| synchronous}4 context of global actor %2",
+      (DescriptiveDeclKind, DeclName, Type, unsigned, bool))
 ERROR(global_actor_from_instance_actor_context,none,
-      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4 from actor %3",
-      (DescriptiveDeclKind, DeclName, Type, DeclName, unsigned))
+      "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
+      " from actor %3 %select{|in a synchronous context}5",
+      (DescriptiveDeclKind, DeclName, Type, DeclName, unsigned, bool))
 ERROR(global_actor_from_other_global_actor_context,none,
       "%0 %1 isolated to global actor %2 can not be %select{referenced|mutated|used 'inout'}4"
-      " from different global actor %3",
-      (DescriptiveDeclKind, DeclName, Type, Type, unsigned))
+      " from different global actor %3 %select{|in a synchronous context}5",
+      (DescriptiveDeclKind, DeclName, Type, Type, unsigned, bool))
 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|an '@actorIndependent'}3 context",
-      (DescriptiveDeclKind, DeclName, Type, bool, unsigned))
+      " from %select{this|an '@actorIndependent'}3%select{| synchronous}5 context",
+      (DescriptiveDeclKind, DeclName, Type, bool, unsigned, bool))
 ERROR(actor_isolated_partial_apply,none,
         "actor-isolated %0 %1 can not be partially applied",
         (DescriptiveDeclKind, DeclName))

lib/SILGen/SILGenExpr.cpp

@@ -2198,7 +2198,7 @@ RValue RValueEmitter::visitMemberRefExpr(MemberRefExpr *e,
 
 RValue RValueEmitter::visitDynamicMemberRefExpr(DynamicMemberRefExpr *E,
                                                 SGFContext C) {
-  assert(!E->isImplicitlyAsync() && "actors do not have @objc members");
+  assert(!E->isImplicitlyAsync() && "an actor-isolated @objc member?");
   return SGF.emitDynamicMemberRefExpr(E, C);
 }
 
@@ -2220,7 +2220,7 @@ RValue RValueEmitter::visitSubscriptExpr(SubscriptExpr *E, SGFContext C) {
 
 RValue RValueEmitter::visitDynamicSubscriptExpr(
                                       DynamicSubscriptExpr *E, SGFContext C) {
-  assert(!E->isImplicitlyAsync() && "actors do not have @objc members");
+  assert(!E->isImplicitlyAsync() && "an actor-isolated @objc member?");
   return SGF.emitDynamicSubscriptExpr(E, C);
 }
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -1558,70 +1558,95 @@ namespace {
       return false;
     }
 
-    /// Check a reference to an entity within a global actor.
-    bool checkGlobalActorReference(
-        ConcreteDeclRef valueRef, SourceLoc loc, Type globalActor,
-        bool isCrossActor,
-        Expr *context) {
-      ValueDecl *value = valueRef.getDecl();
+    enum class AsyncMarkingResult {
+      FoundAsync, // successfully marked an implicitly-async operation
+      NotFound,  // fail: no valid implicitly-async operation was found
+      SyncContext, // fail: a valid implicitly-async op, but in sync context
+      NotConcurrentValue  // fail: valid op and context, but not ConcurrentValue
+    };
 
-      /// Returns true if this global-actor reference is acceptable because
-      /// it is part of an implicitly async operation, such as a call or
-      /// property access.
-      /// NOTE: This check will mutate the AST if it returns true!
-      auto inspectForImplicitlyAsync = [&] () -> bool {
-        // If our current context isn't an asynchronous one, don't
-        if (!isInAsynchronousContext())
-          return false;
+    /// Attempts to identify and mark a valid cross-actor use of a synchronous
+    /// actor-isolated member (e.g., sync function application, property access)
+    AsyncMarkingResult tryMarkImplicitlyAsync(SourceLoc declLoc,
+                                              ConcreteDeclRef concDeclRef,
+                                              Expr* context) {
+      ValueDecl *decl = concDeclRef.getDecl();
+      AsyncMarkingResult result = AsyncMarkingResult::NotFound;
 
-        bool asyncAccess = false;
+      // is it an access to a property?
+      if (isPropOrSubscript(decl)) {
+        if (auto declRef = dyn_cast_or_null<DeclRefExpr>(context)) {
+          if (usageEnv(declRef) == VarRefUseEnv::Read) {
 
-        // Is this global-actor reference part of a LookupExpr or DeclRefExpr?
-        if (isPropOrSubscript(valueRef.getDecl())) {
-          if (auto declRef = dyn_cast_or_null<DeclRefExpr>(context)) {
-            if (usageEnv(declRef) == VarRefUseEnv::Read) {
-              declRef->setImplicitlyAsync(true);
-              asyncAccess = true;
-            }
-          } else if (auto lookupExpr = dyn_cast_or_null<LookupExpr>(context)) {
-            if (usageEnv(lookupExpr) == VarRefUseEnv::Read) {
-              lookupExpr->setImplicitlyAsync(true);
-              asyncAccess = true;
-            }
+            if (!isInAsynchronousContext())
+              return AsyncMarkingResult::SyncContext;
+
+            declRef->setImplicitlyAsync(true);
+            result = AsyncMarkingResult::FoundAsync;
           }
-        }
+        } else if (auto lookupExpr = dyn_cast_or_null<LookupExpr>(context)) {
+          if (usageEnv(lookupExpr) == VarRefUseEnv::Read) {
 
-        // Is this global-actor reference within an apply?
-        if (!applyStack.empty()) {
-          // Check our applyStack metadata from the traversal.
-          // Our goal is to identify whether this global actor reference appears
-          // as the called value of the enclosing ApplyExpr. We cannot simply
-          // inspect Parent here because of expressions like (callee)()
-          // and the fact that the reference may be just an argument to an apply
-          ApplyExpr *apply = applyStack.back();
-          Expr *fn = apply->getFn()->getValueProvidingExpr();
-          if (auto memberRef = findMemberReference(fn)) {
-            auto concDecl = memberRef->first;
-            if (value == concDecl.getDecl() && !apply->implicitlyAsync()) {
-              // then this ValueDecl appears as the called value of the ApplyExpr.
-              markNearestCallAsImplicitlyAsync();
-              asyncAccess = true;
-            }
+            if (!isInAsynchronousContext())
+              return AsyncMarkingResult::SyncContext;
+
+            lookupExpr->setImplicitlyAsync(true);
+            result = AsyncMarkingResult::FoundAsync;
           }
         }
 
-        if (asyncAccess) {
-          // Check for non-concurrent types.
-          (void)diagnoseNonConcurrentTypesInReference(
-              valueRef, getDeclContext(), loc,
-              ConcurrentReferenceKind::SynchronousAsAsyncCall);
+      } else if (llvm::isa_and_nonnull<SelfApplyExpr>(context) &&
+          isa<AbstractFunctionDecl>(decl)) {
+        // actor-isolated non-isolated-self calls are implicitly async
+        // and thus OK.
 
-          return true;
+        if (!isInAsynchronousContext())
+          return AsyncMarkingResult::SyncContext;
+
+        markNearestCallAsImplicitlyAsync();
+        result = AsyncMarkingResult::FoundAsync;
+
+      } else if (!applyStack.empty()) {
+        // Check our applyStack metadata from the traversal.
+        // Our goal is to identify whether the actor reference appears
+        // as the called value of the enclosing ApplyExpr. We cannot simply
+        // inspect Parent here because of expressions like (callee)()
+        // and the fact that the reference may be just an argument to an apply
+        ApplyExpr *apply = applyStack.back();
+        Expr *fn = apply->getFn()->getValueProvidingExpr();
+        if (auto memberRef = findMemberReference(fn)) {
+          auto concDecl = memberRef->first;
+          if (decl == concDecl.getDecl() && !apply->implicitlyAsync()) {
+
+            if (!isInAsynchronousContext())
+              return AsyncMarkingResult::SyncContext;
+
+            // then this ValueDecl appears as the called value of the ApplyExpr.
+            markNearestCallAsImplicitlyAsync();
+            result = AsyncMarkingResult::FoundAsync;
+          }
         }
+      }
 
-        return false;
-      };
+      if (result == AsyncMarkingResult::FoundAsync) {
+        // Check for non-concurrent types.
+        bool problemFound =
+            diagnoseNonConcurrentTypesInReference(
+              concDeclRef, getDeclContext(), declLoc,
+              ConcurrentReferenceKind::SynchronousAsAsyncCall);
+        if (problemFound)
+          result = AsyncMarkingResult::NotConcurrentValue;
+      }
 
+      return result;
+    }
+
+    /// Check a reference to an entity within a global actor.
+    bool checkGlobalActorReference(
+        ConcreteDeclRef valueRef, SourceLoc loc, Type globalActor,
+        bool isCrossActor,
+        Expr *context) {
+      ValueDecl *value = valueRef.getDecl();
       auto declContext = getDeclContext();
 
       // Check whether we are within the same isolation context, in which
@@ -1641,7 +1666,8 @@ namespace {
 
       switch (contextIsolation) {
       case ActorIsolation::ActorInstance: {
-        if (inspectForImplicitlyAsync())
+        auto result = tryMarkImplicitlyAsync(loc, valueRef, context);
+        if (result == AsyncMarkingResult::FoundAsync)
           return false;
 
         auto useKind = static_cast<unsigned>(
@@ -1650,7 +1676,7 @@ namespace {
         ctx.Diags.diagnose(loc, diag::global_actor_from_instance_actor_context,
                            value->getDescriptiveKind(), value->getName(),
                            globalActor, contextIsolation.getActor()->getName(),
-                           useKind);
+                           useKind, result == AsyncMarkingResult::SyncContext);
         noteIsolatedActorMember(value, context);
         return true;
       }
@@ -1659,7 +1685,8 @@ namespace {
       case ActorIsolation::GlobalActorUnsafe: {
         // Check if this decl reference is the callee of the enclosing Apply,
         // making it OK as an implicitly async call.
-        if (inspectForImplicitlyAsync())
+        auto result = tryMarkImplicitlyAsync(loc, valueRef, context);
+        if (result == AsyncMarkingResult::FoundAsync)
           return false;
 
         auto useKind = static_cast<unsigned>(
@@ -1669,7 +1696,8 @@ namespace {
         ctx.Diags.diagnose(
             loc, diag::global_actor_from_other_global_actor_context,
             value->getDescriptiveKind(), value->getName(), globalActor,
-            contextIsolation.getGlobalActor(), useKind);
+            contextIsolation.getGlobalActor(), useKind,
+            result == AsyncMarkingResult::SyncContext);
         noteIsolatedActorMember(value, context);
         return true;
       }
@@ -1679,7 +1707,8 @@ namespace {
         return false;
 
       case ActorIsolation::Independent: {
-        if (inspectForImplicitlyAsync())
+        auto result = tryMarkImplicitlyAsync(loc, valueRef, context);
+        if (result == AsyncMarkingResult::FoundAsync)
           return false;
 
         auto useKind = static_cast<unsigned>(
@@ -1688,14 +1717,16 @@ namespace {
         ctx.Diags.diagnose(loc, diag::global_actor_from_nonactor_context,
                            value->getDescriptiveKind(), value->getName(),
                            globalActor,
-                           /*actorIndependent=*/true, useKind);
+                           /*actorIndependent=*/true, useKind,
+                           result == AsyncMarkingResult::SyncContext);
         noteIsolatedActorMember(value, context);
         return true;
       }
 
       case ActorIsolation::Unspecified: {
         // NOTE: we must always inspect for implicitlyAsync
-        bool implicitlyAsyncExpr = inspectForImplicitlyAsync();
+        auto result = tryMarkImplicitlyAsync(loc, valueRef, context);
+        bool implicitlyAsyncExpr = (result == AsyncMarkingResult::FoundAsync);
         bool didEmitDiagnostic = false;
 
         auto emitError = [&](bool justNote = false) {
@@ -1706,7 +1737,8 @@ namespace {
             ctx.Diags.diagnose(
               loc, diag::global_actor_from_nonactor_context,
               value->getDescriptiveKind(), value->getName(), globalActor,
-              /*actorIndependent=*/false, useKind);
+              /*actorIndependent=*/false, useKind,
+              result == AsyncMarkingResult::SyncContext);
           }
           noteIsolatedActorMember(value, context);
         };
@@ -1938,47 +1970,15 @@ namespace {
       }
 
       case ActorIsolationRestriction::ActorSelf: {
-        /// Local function to check for implicit async promotion.
-        /// returns None if it is not applicable; true if there is an error.
-        auto checkImplicitlyAsync = [&]() -> Optional<bool> {
-          if (!isInAsynchronousContext())
-            return None;
-
-          bool validAccess = false;
-
-          // actor-isolated non-isolated-self calls are implicitly async
-          // and thus OK.
-          if (llvm::isa_and_nonnull<SelfApplyExpr>(context) &&
-              isa<AbstractFunctionDecl>(member)) {
-            markNearestCallAsImplicitlyAsync();
-            validAccess = true;
-
-          } else if (llvm::isa_and_nonnull<LookupExpr>(context) &&
-                    isPropOrSubscript(member) &&
-                    usageEnv(cast<LookupExpr>(context)) == VarRefUseEnv::Read) {
-            cast<LookupExpr>(context)->setImplicitlyAsync(true);
-            validAccess = true;
-          } else {
-            // It's not wrong to have declref context here; simply unimplemented
-            assert(context == nullptr || !isa<DeclRefExpr>(context));
-          }
-
-          if (validAccess) {
-            // Check for non-concurrent types.
-            return diagnoseNonConcurrentTypesInReference(
-                memberRef, getDeclContext(), memberLoc,
-                ConcurrentReferenceKind::SynchronousAsAsyncCall);
-          }
-
-          return None;
-        };
-
         // Must reference actor-isolated state on 'self'.
         auto *selfVar = getReferencedSelf(base);
         if (!selfVar) {
           // Check for implicit async.
-          if (auto result = checkImplicitlyAsync())
-            return *result;
+          auto result = tryMarkImplicitlyAsync(memberLoc, memberRef, context);
+          if (result == AsyncMarkingResult::FoundAsync)
+            return false; // no problems
+          else if (result == AsyncMarkingResult::NotConcurrentValue)
+            return true;
 
           auto useKind = static_cast<unsigned>(
               kindOfUsage(member, context).getValueOr(VarRefUseEnv::Read));
@@ -2017,9 +2017,11 @@ namespace {
             return false;
 
           case ActorIsolation::Independent: {
-            // Check for implicit async.
-            if (auto result = checkImplicitlyAsync())
-              return *result;
+            auto result = tryMarkImplicitlyAsync(memberLoc, memberRef, context);
+            if (result == AsyncMarkingResult::FoundAsync)
+              return false; // no problems
+            else if (result == AsyncMarkingResult::NotConcurrentValue)
+              return true;
 
             // The 'self' is for an actor-independent member, which means
             // we cannot refer to actor-isolated state.
@@ -2033,20 +2035,26 @@ namespace {
           }
 
           case ActorIsolation::GlobalActor:
-          case ActorIsolation::GlobalActorUnsafe:
-            // Check for implicit async.
-            if (auto result = checkImplicitlyAsync())
-              return *result;
+          case ActorIsolation::GlobalActorUnsafe: {
+            auto result = tryMarkImplicitlyAsync(memberLoc, memberRef, context);
+            if (result == AsyncMarkingResult::FoundAsync)
+              return false; // no problems
+            else if (result == AsyncMarkingResult::NotConcurrentValue)
+              return true;
 
             // The 'self' is for a member that's part of a global actor, which
             // means we cannot refer to actor-isolated state.
-            ctx.Diags.diagnose(
-                memberLoc, diag::actor_isolated_global_actor_context,
-                member->getDescriptiveKind(),
-                member->getName(),
-                contextIsolation.getGlobalActor());
+            auto useKind = static_cast<unsigned>(
+                kindOfUsage(member, context).getValueOr(VarRefUseEnv::Read));
+            ctx.Diags.diagnose(memberLoc,
+                               diag::actor_isolated_global_actor_context,
+                               member->getDescriptiveKind(), member->getName(),
+                               contextIsolation.getGlobalActor(), useKind,
+                               result == AsyncMarkingResult::SyncContext
+                               );
             noteIsolatedActorMember(member, context);
             return true;
+          }
         }
         llvm_unreachable("Unhandled actor isolation");
       }

test/Concurrency/actor_call_implicitly_async.swift

@@ -203,6 +203,16 @@ func blender(_ peeler : () -> Void) {
   await (true ? wisk : {n in return})(1)
 }
 
+actor Chain {
+  var next : Chain?
+}
+
+func walkChain(chain : Chain) async {
+  _ = chain.next?.next?.next?.next // expected-error 4 {{property access is 'async' but is not marked with 'await'}}
+  _ = (await chain.next)?.next?.next?.next // expected-error 3 {{property access is 'async' but is not marked with 'await'}}
+  _ = (await chain.next?.next)?.next?.next // expected-error 2 {{property access is 'async' but is not marked with 'await'}}
+}
+
 
 // want to make sure there is no note about implicitly async on this func.
 @BananaActor func rice() async {}