[Concurrency] @preconcurrency import downgrades globals concurrency errors to warnings

Author: sophiapoirier

include/swift/AST/DiagnosticEngine.h

@@ -564,6 +564,16 @@ namespace swift {
     /// emitted as a warning, but a note will still be emitted as a note.
     InFlightDiagnostic &limitBehavior(DiagnosticBehavior limit);
 
+    /// Conditionally prevent the diagnostic from behaving more severely than \p
+    /// limit. If the condition is false, no limit is imposed.
+    InFlightDiagnostic &limitBehaviorIf(bool shouldLimit,
+                                        DiagnosticBehavior limit) {
+      if (!shouldLimit) {
+        return *this;
+      }
+      return limitBehavior(limit);
+    }
+
     /// Limit the diagnostic behavior to warning until the specified version.
     ///
     /// This helps stage in fixes for stricter diagnostics as warnings

lib/Sema/TypeCheckConcurrency.cpp

@@ -829,12 +829,12 @@ static bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
           conformance.getConcrete())->isMissing());
 }
 
-/// Find the import that makes the given nominal declaration available.
+/// Find the import that makes the given declaration available.
 static llvm::Optional<AttributedImport<ImportedModule>>
-findImportFor(NominalTypeDecl *nominal, const DeclContext *fromDC) {
-  // If the nominal type is from the current module, there's no import.
-  auto nominalModule = nominal->getParentModule();
-  if (nominalModule == fromDC->getParentModule())
+findImportFor(const DeclContext *dc, const DeclContext *fromDC) {
+  // If the type is from the current module, there's no import.
+  auto module = dc->getParentModule();
+  if (module == fromDC->getParentModule())
     return llvm::None;
 
   auto fromSourceFile = fromDC->getParentSourceFile();
@@ -843,16 +843,16 @@ findImportFor(NominalTypeDecl *nominal, const DeclContext *fromDC) {
 
   // Look to see if the owning module was directly imported.
   for (const auto &import : fromSourceFile->getImports()) {
-    if (import.module.importedModule == nominalModule)
+    if (import.module.importedModule == module)
       return import;
   }
 
   // Now look for transitive imports.
-  auto &importCache = nominal->getASTContext().getImportCache();
+  auto &importCache = dc->getASTContext().getImportCache();
   for (const auto &import : fromSourceFile->getImports()) {
     auto &importSet = importCache.getImportSet(import.module.importedModule);
     for (const auto &transitive : importSet.getTransitiveImports()) {
-      if (transitive.importedModule == nominalModule) {
+      if (transitive.importedModule == module) {
         return import;
       }
     }
@@ -2931,7 +2931,8 @@ namespace {
     ///
     /// \returns true if we diagnosed the entity, \c false otherwise.
     bool diagnoseReferenceToUnsafeGlobal(ValueDecl *value, SourceLoc loc) {
-      switch (value->getASTContext().LangOpts.StrictConcurrencyLevel) {
+      auto &ctx = value->getASTContext();
+      switch (ctx.LangOpts.StrictConcurrencyLevel) {
       case StrictConcurrency::Minimal:
       case StrictConcurrency::Targeted:
         // Never diagnose.
@@ -2954,17 +2955,32 @@ namespace {
         return false;
 
       // If it's actor-isolated, it's already been dealt with.
-      if (getActorIsolation(value).isActorIsolated())
+      const auto isolation = getActorIsolation(value);
+      if (isolation.isActorIsolated())
         return false;
 
       if (auto attr = value->getAttrs().getAttribute<NonisolatedAttr>();
           attr && attr->isUnsafe()) {
         return false;
       }
 
+      const auto import =
+          findImportFor(var->getDeclContext(), getDeclContext());
+      const bool isPreconcurrencyImport =
+          import && import->options.contains(ImportFlags::Preconcurrency);
+      const auto isPreconcurrencyUnspecifiedIsolation =
+          isPreconcurrencyImport && isolation.isUnspecified();
       ctx.Diags.diagnose(loc, diag::shared_mutable_state_access, value)
-          .warnUntilSwiftVersion(6);
+          .warnUntilSwiftVersionIf(!isPreconcurrencyUnspecifiedIsolation, 6)
+          .limitBehaviorIf(isPreconcurrencyImport, DiagnosticBehavior::Warning)
+          .limitBehaviorIf(!ctx.isSwiftVersionAtLeast(6) &&
+                               isPreconcurrencyUnspecifiedIsolation,
+                           DiagnosticBehavior::Ignore);
       value->diagnose(diag::kind_declared_here, value->getDescriptiveKind());
+      if (const auto sourceFile = getDeclContext()->getParentSourceFile();
+          sourceFile && isPreconcurrencyImport) {
+        sourceFile->setImportUsedPreconcurrency(*import);
+      }
       return true;
     }
 

test/Concurrency/Inputs/GlobalVariables.swift

@@ -0,0 +1,11 @@
+public struct Globals {
+  public static let integerConstant = 0
+  public static var integerMutable = 0
+
+  public static nonisolated(unsafe) let nonisolatedUnsafeIntegerConstant = 0
+  public static nonisolated(unsafe) var nonisolatedUnsafeIntegerMutable = 0
+
+  @MainActor public static var actorInteger = 0
+
+  public init() {}
+}

test/Concurrency/experimental_feature_strictconcurrency.swift

@@ -1,10 +1,14 @@
-// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency -enable-experimental-feature GlobalConcurrency -swift-version 6 -emit-sil -o /dev/null -verify %s
-// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -swift-version 6 -emit-sil -o /dev/null -verify %s
-// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -swift-version 6 -emit-sil -o /dev/null -verify -verify-additional-prefix region-isolation- -enable-experimental-feature RegionBasedIsolation %s
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/GlobalVariables.swiftmodule -module-name GlobalVariables -parse-as-library -strict-concurrency=minimal -swift-version 5 %S/Inputs/GlobalVariables.swift
+// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency -enable-experimental-feature GlobalConcurrency -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify %s
+// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify %s
+// RUN: %target-swift-frontend -disable-availability-checking -parse-as-library -enable-experimental-feature StrictConcurrency=complete -enable-experimental-feature GlobalConcurrency -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -verify-additional-prefix region-isolation- -enable-experimental-feature RegionBasedIsolation %s
 
 // REQUIRES: concurrency
 // REQUIRES: asserts
 
+@preconcurrency import GlobalVariables
+
 class C1 { } // expected-note{{class 'C1' does not conform to the 'Sendable' protocol}}
 class C2 { }
 
@@ -74,6 +78,7 @@ func f() {
   print(TestStatics.immutableExplicitSendable)
   print(TestStatics.immutableInferredSendable)
   print(TestStatics.mutable) // expected-error{{reference to static property 'mutable' is not concurrency-safe because it involves shared mutable state}}
+  print(Globals.actorInteger) // expected-error{{main actor-isolated static property 'actorInteger' can not be referenced from global actor 'TestGlobalActor'}}
 }
 
 func testLocalNonisolatedUnsafe() async {
@@ -85,6 +90,14 @@ func testLocalNonisolatedUnsafe() async {
   print(await task.value)
 }
 
+func testCGlobals() { // expected-note{{add '@MainActor' to make global function 'testCGlobals()' part of global actor 'MainActor'}}
+  let _ = Globals.integerConstant
+  let _ = Globals.integerMutable // expected-warning{{reference to static property 'integerMutable' is not concurrency-safe because it involves shared mutable state}}
+  let _ = Globals.nonisolatedUnsafeIntegerConstant
+  let _ = Globals.nonisolatedUnsafeIntegerMutable
+  let _ = Globals.actorInteger // expected-error{{main actor-isolated static property 'actorInteger' can not be referenced from a non-isolated context}}
+}
+
 @MainActor
 func iterate(stream: AsyncStream<Int>) async {
   nonisolated(unsafe) var it = stream.makeAsyncIterator()