Merge pull request #35927 from DougGregor/concurrentvalue-classes

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -4274,9 +4274,10 @@ ERROR(actor_isolated_from_escaping_closure,none,
 ERROR(local_function_executed_concurrently,none,
       "concurrently-executed %0 %1 must be marked as '@concurrent'",
       (DescriptiveDeclKind, DeclName))
-ERROR(concurrent_mutation_of_local_capture,none,
-      "mutation of captured %0 %1 in concurrently-executing code",
-      (DescriptiveDeclKind, DeclName))
+ERROR(concurrent_access_of_local_capture,none,
+      "%select{mutation of|reference to}0 captured %1 %2 in "
+      "concurrently-executing code",
+      (bool, DescriptiveDeclKind, DeclName))
 NOTE(actor_isolated_sync_func,none,
      "calls to %0 %1 from outside of its actor context are "
      "implicitly asynchronous",
@@ -4323,6 +4324,13 @@ ERROR(concurrent_value_outside_source_file,none,
       "conformance 'ConcurrentValue' must occur in the same source file as "
       "%0 %1; use 'UnsafeConcurrentValue' for retroactive conformance",
       (DescriptiveDeclKind, DeclName))
+ERROR(concurrent_value_open_class,none,
+      "open class %0 cannot conform to `ConcurrentValue`; "
+      "use `UnsafeConcurrentValue`", (DeclName))
+ERROR(concurrent_value_inherit,none,
+      "`ConcurrentValue` class %1 cannot inherit from another class"
+      "%select{| other than 'NSObject'}0",
+      (bool, DeclName))
 
 ERROR(actorindependent_let,none,
       "'@actorIndependent' is meaningless on 'let' declarations because "

include/swift/Basic/LangOptions.h

@@ -247,6 +247,9 @@ namespace swift {
     /// Enable experimental ConcurrentValue checking.
     bool EnableExperimentalConcurrentValueChecking = false;
 
+    /// Enable experimental flow-sensitive concurrent captures.
+    bool EnableExperimentalFlowSensitiveConcurrentCaptures = false;
+
     /// Disable the implicit import of the _Concurrency module.
     bool DisableImplicitConcurrencyModuleImport = false;
 

include/swift/Option/FrontendOptions.td

@@ -226,6 +226,10 @@ def enable_experimental_concurrent_value_checking :
   Flag<["-"], "enable-experimental-concurrent-value-checking">,
   HelpText<"Enable ConcurrentValue checking">;
 
+def enable_experimental_flow_sensitive_concurrent_captures :
+  Flag<["-"], "enable-experimental-flow-sensitive-concurrent-captures">,
+  HelpText<"Enable flow-sensitive concurrent captures">;
+
 def enable_resilience : Flag<["-"], "enable-resilience">,
      HelpText<"Deprecated, use -enable-library-evolution instead">;
 }

lib/Frontend/CompilerInvocation.cpp

@@ -387,6 +387,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     Args.hasArg(OPT_enable_experimental_concurrency);
   Opts.EnableExperimentalConcurrentValueChecking |=
     Args.hasArg(OPT_enable_experimental_concurrent_value_checking);
+  Opts.EnableExperimentalFlowSensitiveConcurrentCaptures |=
+    Args.hasArg(OPT_enable_experimental_flow_sensitive_concurrent_captures);
 
   Opts.DisableImplicitConcurrencyModuleImport |=
     Args.hasArg(OPT_disable_implicit_concurrency_module_import);

lib/Sema/TypeCheckAttr.cpp

@@ -129,6 +129,7 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   IGNORED_ATTR(OriginallyDefinedIn)
   IGNORED_ATTR(NoDerivative)
   IGNORED_ATTR(SpecializeExtension)
+  IGNORED_ATTR(Concurrent)
 #undef IGNORED_ATTR
 
   void visitAlignmentAttr(AlignmentAttr *attr) {
@@ -420,22 +421,6 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
       }
     }
   }
-
-  void visitConcurrentAttr(ConcurrentAttr *attr) {
-    auto VD = dyn_cast<ValueDecl>(D);
-    if (!VD)
-      return;
-
-    auto innermostDC = VD->getInnermostDeclContext();
-    SubstitutionMap subs;
-    if (auto genericEnv = innermostDC->getGenericEnvironmentOfContext()) {
-      subs = genericEnv->getForwardingSubstitutionMap();
-    }
-
-    (void)diagnoseNonConcurrentTypesInReference(
-        ConcreteDeclRef(VD, subs), innermostDC, VD->getLoc(),
-        ConcurrentReferenceKind::ConcurrentFunction);
-  }
 };
 } // end anonymous namespace
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -1605,21 +1605,28 @@ namespace {
           return false;
 
         // Check whether this is a local variable, in which case we can
-        // determine whether it was captured by value.
+        // determine whether it was safe to access concurrently.
         if (auto var = dyn_cast<VarDecl>(value)) {
           auto parent = mutableLocalVarParent[declRefExpr];
 
-          // If we have an immediate load of this variable, or it's a let,
-          // we will separately ensure that this variable is not modified.
-          if (!var->supportsMutation() || parent.dyn_cast<LoadExpr *>()) {
+          // If the variable is immutable, it's fine so long as it involves
+          // ConcurrentValue types.
+          //
+          // When flow-sensitive concurrent captures are enabled, we also
+          // allow reads, depending on a SIL diagnostic pass to identify the
+          // remaining race conditions.
+          if (!var->supportsMutation() ||
+              (ctx.LangOpts.EnableExperimentalFlowSensitiveConcurrentCaptures &&
+               parent.dyn_cast<LoadExpr *>())) {
             return diagnoseNonConcurrentTypesInReference(
                 valueRef, getDeclContext(), loc,
                 ConcurrentReferenceKind::LocalCapture);
           }
 
-          // Otherwise, we have concurrent mutation. Complain.
+          // Otherwise, we have concurrent access. Complain.
           ctx.Diags.diagnose(
-              loc, diag::concurrent_mutation_of_local_capture,
+              loc, diag::concurrent_access_of_local_capture,
+              parent.dyn_cast<LoadExpr *>(),
               var->getDescriptiveKind(), var->getName());
           return true;
         }
@@ -2307,10 +2314,12 @@ void swift::checkConcurrentValueConformance(ProtocolConformance *conformance) {
   if (!nominal)
     return;
 
-  // Actors implicitly conform to ConcurrentValue and protect their state.
   auto classDecl = dyn_cast<ClassDecl>(nominal);
-  if (classDecl && classDecl->isActor())
-    return;
+  if (classDecl) {
+    // Actors implicitly conform to ConcurrentValue and protect their state.
+    if (classDecl->isActor())
+      return;
+  }
 
   // ConcurrentValue can only be used in the same source file.
   auto conformanceDecl = conformanceDC->getAsDecl();
@@ -2322,6 +2331,29 @@ void swift::checkConcurrentValueConformance(ProtocolConformance *conformance) {
     return;
   }
 
+  if (classDecl) {
+    // An open class cannot conform to `ConcurrentValue`.
+    if (classDecl->getFormalAccess() == AccessLevel::Open) {
+      classDecl->diagnose(
+          diag::concurrent_value_open_class, classDecl->getName());
+      return;
+    }
+
+    // A 'ConcurrentValue' class cannot inherit from another class, although
+    // we allow `NSObject` for Objective-C interoperability.
+    if (!isa<InheritedProtocolConformance>(conformance)) {
+      if (auto superclassDecl = classDecl->getSuperclassDecl()) {
+        if (!superclassDecl->isNSObject()) {
+          classDecl->diagnose(
+              diag::concurrent_value_inherit,
+              nominal->getASTContext().LangOpts.EnableObjCInterop,
+              classDecl->getName());
+          return;
+        }
+      }
+    }
+  }
+
   // Stored properties of structs and classes must have
   // ConcurrentValue-conforming types.
   if (isa<StructDecl>(nominal) || classDecl) {

test/Concurrency/Runtime/actor_counters.swift

@@ -61,7 +61,7 @@ func runTest(numCounters: Int, numWorkers: Int, numIterations: Int) async {
   var workers: [Task.Handle<Void>] = []
   for i in 0..<numWorkers {
     workers.append(
-      Task.runDetached {
+      Task.runDetached { [counters] in
         usleep(UInt32.random(in: 0..<100) * 1000)
         await worker(
           identity: i, counters: counters, numIterations: numIterations,

test/Concurrency/actor_isolation.swift

@@ -142,7 +142,7 @@ extension MyActor {
     acceptConcurrentClosure {
       _ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent closure}}
       _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent closure}}
-      _ = localVar // okay
+      _ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
       localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
       _ = localConstant
     }
@@ -159,7 +159,7 @@ extension MyActor {
     @concurrent func localFn1() {
       _ = self.text[0] // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent function}}
       _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent function}}
-      _ = localVar // okay
+      _ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
       localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
       _ = localConstant
     }
@@ -168,7 +168,7 @@ extension MyActor {
       acceptClosure {
         _ = text[0]  // expected-error{{actor-isolated property 'text' cannot be referenced from a concurrent function}}
         _ = self.synchronous() // expected-error{{actor-isolated instance method 'synchronous()' cannot be referenced from a concurrent function}}
-        _ = localVar // okay
+        _ = localVar // expected-error{{reference to captured var 'localVar' in concurrently-executing code}}
         localVar = 25 // expected-error{{mutation of captured var 'localVar' in concurrently-executing code}}
         _ = localConstant
       }
@@ -324,14 +324,18 @@ func testGlobalRestrictions(actor: MyActor) async {
   // Global mutable state cannot be accessed.
   _ = mutableGlobal // expected-warning{{reference to var 'mutableGlobal' is not concurrency-safe because it involves shared mutable state}}
 
-  // Local mutable variables cannot be modified from concurrently-executing
+  // Local mutable variables cannot be accessed from concurrently-executing
   // code.
   var i = 17
   acceptConcurrentClosure {
-    _ = i
+    _ = i // expected-error{{reference to captured var 'i' in concurrently-executing code}}
     i = 42 // expected-error{{mutation of captured var 'i' in concurrently-executing code}}
   }
   print(i)
+
+  acceptConcurrentClosure { [i] in
+    _ = i
+  }
 }
 
 func f() {

test/Concurrency/concurrent_value_checking.swift

@@ -118,7 +118,7 @@ func testConcurrency() {
   }
   acceptConcurrent {
     print(x) // expected-warning{{cannot use let 'x' with a non-concurrent-value type 'NotConcurrent' from concurrently-executed code}}
-    print(y) // expected-warning{{cannot use var 'y' with a non-concurrent-value type 'NotConcurrent' from concurrently-executed code}}
+    print(y) // expected-error{{reference to captured var 'y' in concurrently-executing code}}
   }
 }
 
@@ -147,8 +147,7 @@ class SomeClass: MainActorProto {
 // ConcurrentValue restriction on concurrent functions.
 // ----------------------------------------------------------------------
 
-// FIXME: poor diagnostic
-@concurrent func concurrentFunc() -> NotConcurrent? { nil } // expected-warning{{cannot call function returning non-concurrent-value type 'NotConcurrent?' across actors}}
+@concurrent func concurrentFunc() -> NotConcurrent? { nil }
 
 // ----------------------------------------------------------------------
 // No ConcurrentValue restriction on @concurrent function types.
@@ -222,9 +221,20 @@ class C5: UnsafeConcurrentValue {
 }
 
 class C6: C5 {
-  var y: Int = 0 // still okay
+  var y: Int = 0 // still okay, it's unsafe
 }
 
+class C7<T>: ConcurrentValue { }
+
+class C8: C7<Int> { } // okay
+
+open class C9: ConcurrentValue { } // expected-error{{open class 'C9' cannot conform to `ConcurrentValue`; use `UnsafeConcurrentValue`}}
+
+public class C10: ConcurrentValue { }
+// expected-note@-1{{superclass is declared here}}
+open class C11: C10 { }
+// expected-error@-1{{superclass 'C10' of open class must be open}}
+// expected-error@-2{{open class 'C11' cannot conform to `ConcurrentValue`; use `UnsafeConcurrentValue`}}
 
 // ----------------------------------------------------------------------
 // UnsafeConcurrentValue disabling checking

test/Concurrency/concurrent_value_checking_objc.swift

@@ -0,0 +1,22 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency -enable-experimental-concurrent-value-checking
+
+// REQUIRES: concurrency
+// REQUIRES: objc_interop
+
+import Foundation
+
+class A: NSObject, ConcurrentValue {
+  let x: Int = 5
+}
+
+class B: NSObject, ConcurrentValue {
+  var x: Int = 5 // expected-error{{stored property 'x' of 'ConcurrentValue'-conforming class 'B' is mutable}}
+}
+
+class C { }
+
+class D: NSObject, ConcurrentValue {
+  let c: C = C() // expected-error{{stored property 'c' of 'ConcurrentValue'-conforming class 'D' has non-concurrent-value type 'C'}}
+}
+
+

test/Concurrency/concurrentfunction_capturediagnostics.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -enable-experimental-concurrency -verify -emit-sil %s -o - >/dev/null
+// RUN: %target-swift-frontend -enable-experimental-concurrency -enable-experimental-flow-sensitive-concurrent-captures -verify -emit-sil %s -o - >/dev/null
 
 // REQUIRES: concurrency
 

test/SILGen/concurrent_functions.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -emit-silgen %s -module-name test -swift-version 5 -enable-experimental-concurrency | %FileCheck %s
+// RUN: %target-swift-frontend -emit-silgen %s -module-name test -swift-version 5 -enable-experimental-concurrency -enable-experimental-flow-sensitive-concurrent-captures | %FileCheck %s
 // REQUIRES: concurrency
 
 func acceptsConcurrent(_: @escaping @concurrent () -> Int) { }

test/attr/attr_concurrent.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -typecheck -verify %s -enable-experimental-concurrency
+// RUN: %target-swift-frontend -typecheck -verify %s -enable-experimental-concurrency -enable-experimental-flow-sensitive-concurrent-captures
 // REQUIRES: concurrency
 
 // Concurrent attribute on a function type.