Merge pull request swiftlang#35915 from DougGregor/concurrentvalue-functions

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4277,8 +4277,6 @@ ERROR(local_function_executed_concurrently,none,
 ERROR(concurrent_mutation_of_local_capture,none,
       "mutation of captured %0 %1 in concurrently-executing code",
       (DescriptiveDeclKind, DeclName))
-NOTE(concurrent_access_here,none,
-     "access in concurrently-executed code here", ())
 NOTE(actor_isolated_sync_func,none,
      "calls to %0 %1 from outside of its actor context are "
      "implicitly asynchronous",
@@ -4314,10 +4312,6 @@ WARNING(non_concurrent_property_type,none,
         "cannot use %0 %1 with a non-concurrent-value type %2 "
         "%select{across actors|from concurrently-executed code}3",
         (DescriptiveDeclKind, DeclName, Type, bool))
-WARNING(non_concurrent_function_type,none,
-        "`@concurrent` %select{function type|closure}0 has "
-        "non-concurrent-value %select{parameter|result}1 type %2",
-        (bool, bool, Type))
 ERROR(non_concurrent_type_member,none,
       "%select{stored property %1|associated value %1}0 of "
       "'ConcurrentValue'-conforming %2 %3 has non-concurrent-value type %4",

lib/Sema/TypeCheckConcurrency.cpp

@@ -520,14 +520,8 @@ ActorIsolationRestriction ActorIsolationRestriction::forDeclaration(
   case DeclKind::Constructor:
   case DeclKind::Func:
   case DeclKind::Subscript: {
-    // Local captures can only be referenced in their local context or a
-    // context that is guaranteed not to run concurrently with it.
+    // Local captures are checked separately.
     if (cast<ValueDecl>(decl)->isLocalCapture()) {
-      // Local functions are safe to capture; their bodies are checked based on
-      // where that capture is used.
-      if (isa<FuncDecl>(decl))
-        return forUnrestricted();
-
       return forLocalCapture(decl->getDeclContext());
     }
 
@@ -911,7 +905,9 @@ bool swift::diagnoseNonConcurrentTypesInReference(
   }
 
   if (auto var = dyn_cast<VarDecl>(declRef.getDecl())) {
-    Type propertyType = var->getValueInterfaceType().subst(subs);
+    Type propertyType = var->isLocalCapture()
+        ? var->getType()
+        : var->getValueInterfaceType().subst(subs);
     if (!isConcurrentValueType(dc, propertyType)) {
       return diagnoseNonConcurrentProperty(loc, refKind, var, propertyType);
     }
@@ -938,69 +934,14 @@ bool swift::diagnoseNonConcurrentTypesInReference(
   return false;
 }
 
-bool swift::diagnoseNonConcurrentTypesInFunctionType(
-    const AnyFunctionType *fnType, const DeclContext *dc, SourceLoc loc,
-    bool isClosure) {
-  ASTContext &ctx = dc->getASTContext();
-  // Bail out immediately if we aren't supposed to do this checking.
-  if (!dc->getASTContext().LangOpts.EnableExperimentalConcurrentValueChecking)
-    return false;
-
-  // Check parameter types.
-  for (const auto &param : fnType->getParams()) {
-    Type paramType = param.getParameterType();
-    if (!isConcurrentValueType(dc, paramType)) {
-      ctx.Diags.diagnose(
-          loc, diag::non_concurrent_function_type, isClosure, false, paramType);
-      return true;
-    }
-  }
-
-  // Check result type.
-  if (!isConcurrentValueType(dc, fnType->getResult())) {
-    ctx.Diags.diagnose(
-        loc, diag::non_concurrent_function_type, isClosure, true,
-        fnType->getResult());
-    return true;
-  }
-
-  return false;
-}
-
 namespace {
-  /// Check whether a particular context may execute concurrently within
-  /// another context.
-  class ConcurrentExecutionChecker {
-    /// Keeps track of the first location at which a given local function is
-    /// referenced from a context that may execute concurrently with the
-    /// context in which it was introduced.
-    llvm::SmallDenseMap<const FuncDecl *, SourceLoc, 4> concurrentRefs;
-
-  public:
-    /// Determine whether (and where) a given local function is referenced
-    /// from a context that may execute concurrently with the context in
-    /// which it is declared.
-    ///
-    /// \returns the source location of the first reference to the local
-    /// function that may be concurrent. If the result is an invalid
-    /// source location, there are no such references.
-    SourceLoc getConcurrentReferenceLoc(const FuncDecl *localFunc);
-
-    /// Determine whether code in the given use context might execute
-    /// concurrently with code in the definition context.
-    bool mayExecuteConcurrentlyWith(
-      const DeclContext *useContext, const DeclContext *defContext);
-  };
-
   /// Check for adherence to the actor isolation rules, emitting errors
   /// when actor-isolated declarations are used in an unsafe manner.
   class ActorIsolationChecker : public ASTWalker {
     ASTContext &ctx;
     SmallVector<const DeclContext *, 4> contextStack;
     SmallVector<ApplyExpr*, 4> applyStack;
 
-    ConcurrentExecutionChecker concurrentExecutionChecker;
-
     using MutableVarSource = llvm::PointerUnion<DeclRefExpr *, InOutExpr *>;
     using MutableVarParent = llvm::PointerUnion<InOutExpr *, LoadExpr *>;
 
@@ -1016,10 +957,7 @@ namespace {
     /// Determine whether code in the given use context might execute
     /// concurrently with code in the definition context.
     bool mayExecuteConcurrentlyWith(
-      const DeclContext *useContext, const DeclContext *defContext) {
-      return concurrentExecutionChecker.mayExecuteConcurrentlyWith(
-          useContext, defContext);
-    }
+        const DeclContext *useContext, const DeclContext *defContext);
 
     /// If the subexpression is a reference to a mutable local variable from a
     /// different context, record its parent. We'll query this as part of
@@ -1148,18 +1086,6 @@ namespace {
       if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
         closure->setActorIsolation(determineClosureIsolation(closure));
         contextStack.push_back(closure);
-
-        
-        // Concurrent closures must be composed of concurrent-safe parameter
-        // and result types.
-        if (isConcurrentClosure(closure)) {
-          if (auto fnType = closure->getType()->getAs<FunctionType>()) {
-            (void)diagnoseNonConcurrentTypesInFunctionType(
-                 fnType, getDeclContext(), closure->getLoc(),
-                 /*isClosure=*/true);
-          }
-        }
-
         return { true, expr };
       }
 
@@ -1689,6 +1615,22 @@ namespace {
           return true;
         }
 
+        if (auto func = dyn_cast<FuncDecl>(value)) {
+          if (func->isConcurrent())
+            return false;
+
+          func->diagnose(
+              diag::local_function_executed_concurrently,
+              func->getDescriptiveKind(), func->getName())
+            .fixItInsert(func->getAttributeInsertionLoc(false), "@concurrent ");
+
+          // Add the @concurrent attribute implicitly, so we don't diagnose
+          // again.
+          const_cast<FuncDecl *>(func)->getAttrs().add(
+              new (ctx) ConcurrentAttr(true));
+          return true;
+        }
+
         // Concurrent access to some other local.
         ctx.Diags.diagnose(
             loc, diag::concurrent_access_local,
@@ -1910,111 +1852,7 @@ namespace {
   };
 }
 
-SourceLoc ConcurrentExecutionChecker::getConcurrentReferenceLoc(
-    const FuncDecl *localFunc) {
-
-  // If we've already computed a result, we're done.
-  auto known = concurrentRefs.find(localFunc);
-  if (known != concurrentRefs.end())
-    return known->second;
-
-  // Record that there are no concurrent references to this local function. This
-  // prevents infinite recursion if two local functions call each other.
-  concurrentRefs[localFunc] = SourceLoc();
-
-  class ConcurrentLocalRefWalker : public ASTWalker {
-    ConcurrentExecutionChecker &checker;
-    const FuncDecl *targetFunc;
-    SmallVector<const DeclContext *, 4> contextStack;
-
-    const DeclContext *getDeclContext() const {
-      return contextStack.back();
-    }
-
-  public:
-    ConcurrentLocalRefWalker(
-      ConcurrentExecutionChecker &checker, const FuncDecl *targetFunc
-    ) : checker(checker), targetFunc(targetFunc) {
-      contextStack.push_back(targetFunc->getDeclContext());
-    }
-
-    std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
-      if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
-        contextStack.push_back(closure);
-        return { true, expr };
-      }
-
-      if (auto *declRef = dyn_cast<DeclRefExpr>(expr)) {
-        // If this is a reference to the target function from a context
-        // that may execute concurrently with the context where the target
-        // function was declared, record the location.
-        if (declRef->getDecl() == targetFunc &&
-            checker.mayExecuteConcurrentlyWith(
-              getDeclContext(), contextStack.front())) {
-          SourceLoc &loc = checker.concurrentRefs[targetFunc];
-          if (loc.isInvalid())
-            loc = declRef->getLoc();
-
-          return { false, expr };
-        }
-
-        return { true, expr };
-      }
-
-      return { true, expr };
-    }
-
-    Expr *walkToExprPost(Expr *expr) override {
-      if (auto *closure = dyn_cast<AbstractClosureExpr>(expr)) {
-        assert(contextStack.back() == closure);
-        contextStack.pop_back();
-      }
-
-      return expr;
-    }
-
-    bool walkToDeclPre(Decl *decl) override {
-      if (isa<NominalTypeDecl>(decl) || isa<ExtensionDecl>(decl))
-        return false;
-
-      if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
-        contextStack.push_back(func);
-      }
-
-      return true;
-    }
-
-    bool walkToDeclPost(Decl *decl) override {
-      if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
-        assert(contextStack.back() == func);
-        contextStack.pop_back();
-      }
-
-      return true;
-    }
-  };
-
-  // Walk the body of the enclosing function, where all references to the
-  // given local function would occur.
-  Stmt *enclosingBody = nullptr;
-  DeclContext *enclosingDC = localFunc->getDeclContext();
-  if (auto enclosingFunc = dyn_cast<AbstractFunctionDecl>(enclosingDC))
-    enclosingBody = enclosingFunc->getBody();
-  else if (auto enclosingClosure = dyn_cast<ClosureExpr>(enclosingDC))
-    enclosingBody = enclosingClosure->getBody();
-  else if (auto enclosingTopLevelCode = dyn_cast<TopLevelCodeDecl>(enclosingDC))
-    enclosingBody = enclosingTopLevelCode->getBody();
-  else
-    return SourceLoc();
-
-  assert(enclosingBody && "Cannot have a local function here");
-  ConcurrentLocalRefWalker walker(*this, localFunc);
-  enclosingBody->walk(walker);
-
-  return concurrentRefs[localFunc];
-}
-
-bool ConcurrentExecutionChecker::mayExecuteConcurrentlyWith(
+bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
     const DeclContext *useContext, const DeclContext *defContext) {
   // Walk the context chain from the use to the definition.
   while (useContext != defContext) {
@@ -2029,25 +1867,6 @@ bool ConcurrentExecutionChecker::mayExecuteConcurrentlyWith(
         // If the function is @concurrent... it can be run concurrently.
         if (func->isConcurrent())
           return true;
-
-        // If we find a local function that was referenced in code that can be
-        // executed concurrently with where the local function was declared, the
-        // local function can be run concurrently.
-        SourceLoc concurrentLoc = getConcurrentReferenceLoc(func);
-        if (concurrentLoc.isValid()) {
-          ASTContext &ctx = func->getASTContext();
-          func->diagnose(
-              diag::local_function_executed_concurrently,
-              func->getDescriptiveKind(), func->getName())
-            .fixItInsert(func->getAttributeInsertionLoc(false), "@concurrent ");
-          ctx.Diags.diagnose(concurrentLoc, diag::concurrent_access_here);
-
-          // Add the @concurrent attribute implicitly, so we don't diagnose
-          // again.
-          const_cast<FuncDecl *>(func)->getAttrs().add(
-              new (ctx) ConcurrentAttr(true));
-          return true;
-        }
       }
     }
 

lib/Sema/TypeCheckConcurrency.h

@@ -237,12 +237,6 @@ bool diagnoseNonConcurrentTypesInReference(
     ConcreteDeclRef declRef, const DeclContext *dc, SourceLoc loc,
     ConcurrentReferenceKind refKind);
 
-/// Diagnose the presence of any non-concurrent types within the given
-/// function type.
-bool diagnoseNonConcurrentTypesInFunctionType(
-    const AnyFunctionType *fnType, const DeclContext *dc, SourceLoc loc,
-    bool isClosure);
-
 /// Check the correctness of the given ConcurrentValue conformance.
 void checkConcurrentValueConformance(ProtocolConformance *conformance);
 

lib/Sema/TypeCheckType.cpp

@@ -2823,14 +2823,6 @@ NeverNullType TypeResolver::resolveASTFunctionType(
   if (fnTy->hasError())
     return fnTy;
 
-  // Concurrent function types must be composed of concurrent-safe parameter
-  // and result types.
-  if (concurrent && resolution.getStage() > TypeResolutionStage::Structural) {
-    (void)diagnoseNonConcurrentTypesInFunctionType(
-         fnTy, resolution.getDeclContext(), repr->getLoc(),
-         /*isClosure=*/false);
-  }
-
   // If the type is a block or C function pointer, it must be representable in
   // ObjC.
   switch (representation) {

test/Concurrency/actor_isolation.swift

@@ -356,7 +356,7 @@ func checkLocalFunctions() async {
   }
 
   func local2() { // expected-error{{concurrently-executed local function 'local2()' must be marked as '@concurrent'}}{{3-3=@concurrent }}
-    j = 42 // expected-error{{mutation of captured var 'j' in concurrently-executing code}}
+    j = 42
   }
 
   // Okay to call locally.
@@ -371,7 +371,7 @@ func checkLocalFunctions() async {
 
   // Escaping closures can make the local function execute concurrently.
   acceptConcurrentClosure {
-    local2() // expected-note{{access in concurrently-executed code here}}
+    local2()
   }
 
   print(i)
@@ -380,7 +380,7 @@ func checkLocalFunctions() async {
   var k = 17
   func local4() {
     acceptConcurrentClosure {
-      local3() // expected-note{{access in concurrently-executed code here}}
+      local3()
     }
   }
 

test/Concurrency/async_task_groups.swift

@@ -87,8 +87,8 @@ func first_allMustSucceed() async throws {
 }
 
 func first_ignoreFailures() async throws {
-  func work() async -> Int { 42 }
-  func boom() async throws -> Int { throw Boom() }
+  @concurrent func work() async -> Int { 42 }
+  @concurrent func boom() async throws -> Int { throw Boom() }
 
   let first: Int = try await Task.withGroup(resultType: Int.self) { group in
     await group.add { await work() }