Merge pull request #42445 from DougGregor/strict-concurrency-flag

DougGregor · web-flow · commit 12a11e541644 · 2022-04-19T08:07:30.000-07:00
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -58,6 +58,19 @@ namespace swift {
     Complete,
   };
 
+  /// Describes how strict concurrency checking should be.
+  enum class StrictConcurrency {
+    /// Turns off strict checking, which disables (e.g.) Sendable checking in
+    /// most cases.
+    Off,
+    /// Enables concurrency checking in a limited manner that is intended to
+    /// only affect code that has already adopted the concurrency model.
+    Limited,
+    /// Enables strict concurrency checking throughout the entire model,
+    /// providing an approximation of the fully-checked model.
+    On
+  };
+
   /// Access or distribution level of a library.
   enum class LibraryLevel : uint8_t {
     /// Application Programming Interface that is publicly distributed so
@@ -310,11 +323,8 @@ namespace swift {
     /// optimized custom allocator, so that memory debugging tools can be used.
     bool UseMalloc = false;
 
-    /// Provide additional warnings about code that is unsafe in the
-    /// eventual Swift concurrency model, and will eventually become errors
-    /// in a future Swift language version, but are too noisy for existing
-    /// language modes.
-    bool WarnConcurrency = false;
+    /// Specifies how strict concurrency checking will be.
+    StrictConcurrency StrictConcurrencyLevel = StrictConcurrency::Limited;
 
     /// Enable experimental #assert feature.
     bool EnableExperimentalStaticAssert = false;
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
@@ -699,6 +699,14 @@ def warn_concurrency : Flag<["-"], "warn-concurrency">,
   HelpText<"Warn about code that is unsafe according to the Swift Concurrency "
            "model and will become ill-formed in a future language version">;
 
+def strict_concurrency : Joined<["-"], "strict-concurrency=">,
+  Flags<[FrontendOption, ModuleInterfaceOptionIgnorable]>,
+  HelpText<"Specify the how strict concurrency checking will be. The value may "
+           "be 'off' (most 'Sendable' checking is disabled), "
+           "'limited' ('Sendable' checking is enabled in code that uses the "
+           "concurrency model, or 'on' ('Sendable' and other checking is "
+           "enabled for all code in the module)">;
+
 def Rpass_EQ : Joined<["-"], "Rpass=">,
   Flags<[FrontendOption]>,
   HelpText<"Report performed transformations by optimization passes whose "
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -9200,7 +9200,9 @@ ActorIsolation swift::getActorIsolationOfContext(DeclContext *dc) {
   }
 
   if (auto *tld = dyn_cast<TopLevelCodeDecl>(dc)) {
-    if (dc->isAsyncContext() || dc->getASTContext().LangOpts.WarnConcurrency) {
+    if (dc->isAsyncContext() ||
+        dc->getASTContext().LangOpts.StrictConcurrencyLevel
+            >= StrictConcurrency::On) {
       if (Type mainActor = dc->getASTContext().getMainActorType())
         return ActorIsolation::forGlobalActor(
             mainActor,
diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
@@ -235,6 +235,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
                        options::OPT_enable_actor_data_race_checks,
                        options::OPT_disable_actor_data_race_checks);
   inputArgs.AddLastArg(arguments, options::OPT_warn_concurrency);
+  inputArgs.AddLastArg(arguments, options::OPT_strict_concurrency);
   inputArgs.AddLastArg(arguments, options::OPT_warn_implicit_overrides);
   inputArgs.AddLastArg(arguments, options::OPT_typo_correction_limit);
   inputArgs.AddLastArg(arguments, options::OPT_enable_app_extension);
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -692,7 +692,28 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     }
   }
 
-  Opts.WarnConcurrency |= Args.hasArg(OPT_warn_concurrency);
+  // Swift 6+ uses the strictest concurrency level.
+  if (Opts.isSwiftVersionAtLeast(6)) {
+    Opts.StrictConcurrencyLevel = StrictConcurrency::On;
+  } else if (const Arg *A = Args.getLastArg(OPT_strict_concurrency)) {
+    auto value = llvm::StringSwitch<Optional<StrictConcurrency>>(A->getValue())
+      .Case("off", StrictConcurrency::Off)
+      .Case("limited", StrictConcurrency::Limited)
+      .Case("on", StrictConcurrency::On)
+      .Default(None);
+
+    if (value)
+      Opts.StrictConcurrencyLevel = *value;
+    else
+      Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
+                     A->getAsString(Args), A->getValue());
+
+  } else if (Args.hasArg(OPT_warn_concurrency)) {
+    Opts.StrictConcurrencyLevel = StrictConcurrency::On;
+  } else {
+    // Default to "limited" checking in Swift 5.x.
+    Opts.StrictConcurrencyLevel = StrictConcurrency::Limited;
+  }
 
   Opts.WarnImplicitOverrides =
     Args.hasArg(OPT_warn_implicit_overrides);
diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp
@@ -1018,8 +1018,8 @@ ModuleDecl *CompilerInstance::getMainModule() const {
     }
     if (Invocation.getFrontendOptions().EnableLibraryEvolution)
       MainModule->setResilienceStrategy(ResilienceStrategy::Resilient);
-    if (Invocation.getLangOptions().WarnConcurrency ||
-        Invocation.getLangOptions().isSwiftVersionAtLeast(6))
+    if (Invocation.getLangOptions().StrictConcurrencyLevel
+            >= StrictConcurrency::On)
       MainModule->setIsConcurrencyChecked(true);
 
     // Register the main module with the AST context.
diff --git a/lib/IDE/CompletionLookup.cpp b/lib/IDE/CompletionLookup.cpp
@@ -750,8 +750,7 @@ void CompletionLookup::analyzeActorIsolation(
     // For "unsafe" global actor isolation, automatic 'async' only happens
     // if the context has adopted concurrency.
     if (!CanCurrDeclContextHandleAsync &&
-        !completionContextUsesConcurrencyFeatures(CurrDeclContext) &&
-        !CurrDeclContext->getParentModule()->isConcurrencyChecked()) {
+        !completionContextUsesConcurrencyFeatures(CurrDeclContext)) {
       return;
     }
     LLVM_FALLTHROUGH;
@@ -769,7 +768,7 @@ void CompletionLookup::analyzeActorIsolation(
   }
 
   // If the reference is 'async', all types must be 'Sendable'.
-  if (CurrDeclContext->getParentModule()->isConcurrencyChecked() &&
+  if (Ctx.LangOpts.StrictConcurrencyLevel >= StrictConcurrency::On &&
       implicitlyAsync && T) {
     auto *M = CurrDeclContext->getParentModule();
     if (isa<VarDecl>(VD)) {
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -346,7 +346,8 @@ GlobalActorAttributeRequest::evaluate(
       // ... but not if it's an async-context top-level global
       if (var->isTopLevelGlobal() &&
           (var->getDeclContext()->isAsyncContext() ||
-           var->getASTContext().LangOpts.WarnConcurrency)) {
+           var->getASTContext().LangOpts.StrictConcurrencyLevel >=
+             StrictConcurrency::On)) {
         var->diagnose(diag::global_actor_top_level_var)
             .highlight(globalActorAttr->getRangeWithAt());
         return None;
@@ -607,9 +608,6 @@ static bool hasUnavailableConformance(ProtocolConformanceRef conformance) {
 }
 
 static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
-  if (dc->getParentModule()->isConcurrencyChecked())
-    return true;
-
   return contextRequiresStrictConcurrencyChecking(dc, [](const AbstractClosureExpr *) {
     return Type();
   });
@@ -2117,9 +2115,16 @@ namespace {
     ///
     /// \returns true if we diagnosed the entity, \c false otherwise.
     bool diagnoseReferenceToUnsafeGlobal(ValueDecl *value, SourceLoc loc) {
-      if (!getDeclContext()->getParentModule()->isConcurrencyChecked())
+      switch (value->getASTContext().LangOpts.StrictConcurrencyLevel) {
+      case StrictConcurrency::Off:
+      case StrictConcurrency::Limited:
+        // Never diagnose.
         return false;
 
+      case StrictConcurrency::On:
+        break;
+      }
+
       // Only diagnose direct references to mutable global state.
       auto var = dyn_cast<VarDecl>(value);
       if (!var || var->isLet())
@@ -3683,7 +3688,8 @@ ActorIsolation ActorIsolationRequest::evaluate(
 
   if (auto var = dyn_cast<VarDecl>(value)) {
     if (var->isTopLevelGlobal() &&
-        (var->getASTContext().LangOpts.WarnConcurrency ||
+        (var->getASTContext().LangOpts.StrictConcurrencyLevel >=
+             StrictConcurrency::On ||
          var->getDeclContext()->isAsyncContext())) {
       if (Type mainActor = var->getASTContext().getMainActorType())
         return inferredIsolation(
@@ -3891,10 +3897,16 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
 bool swift::contextRequiresStrictConcurrencyChecking(
     const DeclContext *dc,
     llvm::function_ref<Type(const AbstractClosureExpr *)> getType) {
-  // If Swift >= 6, everything uses strict concurrency checking.
-  if (dc->getASTContext().LangOpts.isSwiftVersionAtLeast(6))
+  switch (dc->getASTContext().LangOpts.StrictConcurrencyLevel) {
+  case StrictConcurrency::On:
     return true;
 
+  case StrictConcurrency::Limited:
+  case StrictConcurrency::Off:
+    // Check below to see if the context has adopted concurrency features.
+    break;
+  }
+
   while (!dc->isModuleScopeContext()) {
     if (auto closure = dyn_cast<AbstractClosureExpr>(dc)) {
       // A closure with an explicit global actor or nonindependent
diff --git a/test/ClangImporter/objc_async.swift b/test/ClangImporter/objc_async.swift
@@ -1,10 +1,9 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/custom-modules %s -verify -verify-additional-file %swift_src_root/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h -warn-concurrency -parse-as-library
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/custom-modules %s -verify -verify-additional-file %swift_src_root/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h -strict-concurrency=limited -parse-as-library
 
 // REQUIRES: objc_interop
 // REQUIRES: concurrency
 import Foundation
 import ObjCConcurrency
-// expected-remark@-1{{add '@preconcurrency' to suppress 'Sendable'-related warnings from module 'ObjCConcurrency'}}
 
 @available(SwiftStdlib 5.5, *)
 @MainActor func onlyOnMainActor() { }
@@ -358,8 +357,6 @@ func testSender(
   // expected-warning@-1 {{conformance of 'NonSendableClass' to 'Sendable' is unavailable}}
 
   sender.sendGeneric(sendableGeneric)
-  // expected-warning@-1 {{type 'GenericObject<SendableClass>' does not conform to the 'Sendable' protocol}}
-  // FIXME(rdar://89992569): Should allow for the possibility that GenericObject will have a Sendable subclass
   sender.sendGeneric(nonSendableGeneric)
   // expected-error@-1 {{argument type 'GenericObject<SendableClass>' does not conform to expected type 'Sendable'}}
   // FIXME(rdar://89992095): Should be a warning because we're in -warn-concurrency
diff --git a/test/Concurrency/actor_isolation.swift b/test/Concurrency/actor_isolation.swift
@@ -463,7 +463,7 @@ func testGlobalActorClosures() {
     return 17
   }
 
-  acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-warning{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}
+  acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-error{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}
 }
 
 @available(SwiftStdlib 5.1, *)
diff --git a/test/Concurrency/concurrent_value_checking.swift b/test/Concurrency/concurrent_value_checking.swift
@@ -174,7 +174,7 @@ func testConcurrency() {
 func testUnsafeSendableNothing() {
   var x = 5
   acceptUnsafeSendable {
-    x = 17
+    x = 17 // expected-error{{mutation of captured var 'x' in concurrently-executing code}}
   }
   print(x)
 }
diff --git a/test/Concurrency/toplevel/no-async-6-top-level.swift b/test/Concurrency/toplevel/no-async-6-top-level.swift
@@ -1,4 +1,3 @@
-// RUN: %target-swift-frontend -typecheck -disable-availability-checking -enable-experimental-async-top-level -swift-version 6 %s -verify
 // RUN: %target-swift-frontend -typecheck -disable-availability-checking -swift-version 6 %s -verify
 
 // REQUIRES: asserts
@@ -8,45 +7,49 @@
 // context. `a` is just a normal top-level global variable with no actor
 // isolation.
 
-var a = 10 // expected-note 15 {{var declared here}}
+var a = 10 // expected-note 2 {{var declared here}}
+// expected-note@-1 2{{mutation of this var is only permitted within the actor}}
 
+// expected-note@+1 3{{add '@MainActor' to make global function 'nonIsolatedSync()' part of global actor 'MainActor'}}
 func nonIsolatedSync() {
-    print(a) // expected-warning {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
-    a = a + 10 // expected-warning 2 {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+    print(a) // expected-error {{main actor-isolated var 'a' can not be referenced from a non-isolated context}}
+    a = a + 10 // expected-error{{main actor-isolated var 'a' can not be referenced from a non-isolated context}}
+  // expected-error@-1{{main actor-isolated var 'a' can not be mutated from a non-isolated context}}
 }
 
 @MainActor
-func isolatedSync() { // expected-note 2 {{calls to global function 'isolatedSync()' from outside of its actor context are implicitly asynchronous}}
-    print(a) // expected-warning {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
-    a = a + 10 // expected-warning 2 {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+func isolatedSync() {
+    print(a)
+    a = a + 10
 }
 
 func nonIsolatedAsync() async {
-    await print(a) // expected-warning {{no 'async' operations occur within 'await' expression}}
-    // expected-warning@-1 {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
-    a = a + 10 // expected-warning 2 {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+  await print(a)
+  a = a + 10 // expected-error{{main actor-isolated var 'a' can not be mutated from a non-isolated context}}
+  // expected-note@-1{{property access is 'async'}}
+  // expected-error@-2{{expression is 'async' but is not marked with 'await'}}
 }
 
 @MainActor
-func isolatedAsync() async { // expected-note 2 {{calls to global function 'isolatedAsync()' from outside of its actor context are implicitly asynchronous}}
-    print(a) // expected-warning {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
-    a = a + 10 // expected-warning 2 {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+func isolatedAsync() async {
+    print(a)
+    a = a + 10
 }
 
 nonIsolatedSync()
-isolatedSync() // expected-error {{call to main actor-isolated global function 'isolatedSync()' in a synchronous nonisolated context}}
+isolatedSync()
 nonIsolatedAsync() // expected-error {{'async' call in a function that does not support concurrency}}
-isolatedAsync() // expected-error {{call to main actor-isolated global function 'isolatedAsync()' in a synchronous nonisolated context}}
+isolatedAsync()
 // expected-error@-1 {{'async' call in a function that does not support concurrency}}
 
-print(a) // expected-warning {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+print(a)
 
-if a > 10 { // expected-warning {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+if a > 10 {
     nonIsolatedSync()
-    isolatedSync() // expected-error {{call to main actor-isolated global function 'isolatedSync()' in a synchronous nonisolated context}}
+    isolatedSync()
     nonIsolatedAsync() // expected-error {{'async' call in a function that does not support concurrency}}
-    isolatedAsync() // expected-error {{call to main actor-isolated global function 'isolatedAsync()' in a synchronous nonisolated context}}
+    isolatedAsync()
     // expected-error@-1 {{'async' call in a function that does not support concurrency}}
 
-    print(a) // expected-warning {{reference to var 'a' is not concurrency-safe because it involves shared mutable state}}
+    print(a)
 }
diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp
@@ -4281,7 +4281,7 @@ int main(int argc, char *argv[]) {
     InitInvok.getLangOptions().EnableExperimentalConcurrency = true;
   }
   if (options::WarnConcurrency) {
-    InitInvok.getLangOptions().WarnConcurrency = true;
+    InitInvok.getLangOptions().StrictConcurrencyLevel = StrictConcurrency::On;
   }
   if (options::DisableImplicitConcurrencyImport) {
     InitInvok.getLangOptions().DisableImplicitConcurrencyModuleImport = true;

Original file line number	Diff line number	Diff line change
`@@ -463,7 +463,7 @@ func testGlobalActorClosures() {`
`463`	`463`	`return 17`
`464`	`464`	`}`
`465`	`465`
`466`		`- acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-warning{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}`
	`466`	`+ acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-error{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}`
`467`	`467`	`}`
`468`	`468`
`469`	`469`	`@available(SwiftStdlib 5.1, *)`