Merge pull request swiftlang#73698 from DougGregor/preconcurrency-fixes

Various fixes for `@preconcurrency`

Author: DougGregor

include/swift/AST/DiagnosticsSIL.def

@@ -694,9 +694,6 @@ ERROR(isolated_after_nonisolated, none,
 NOTE(nonisolated_blame, none, "after %1%2 %3, "
      "only non-isolated properties of 'self' can be accessed from "
      "%select{this init|a deinit}0", (bool, StringRef, StringRef, DeclName))
-ERROR(non_sendable_from_deinit,none,
-        "cannot access %1 %2 with a non-sendable type %0 from non-isolated deinit",
-        (Type, DescriptiveDeclKind, DeclName))
 ERROR(isolated_property_mutation_in_nonisolated_context,none,
       "actor-isolated %kind0 can not be %select{referenced|mutated}1 "
       "from a non-isolated context",

include/swift/AST/DiagnosticsSema.def

@@ -5520,8 +5520,8 @@ ERROR(sendable_isolated_sync_function,none,
       "%0 synchronous %kind1 cannot be marked as '@Sendable'",
       (ActorIsolation, const ValueDecl *))
 ERROR(nonsendable_instance_method,none,
-      "instance methods of non-Sendable types cannot be marked as '@Sendable'",
-      ())
+      "instance method of non-Sendable type %0 cannot be marked as '@Sendable'",
+      (Type))
 ERROR(concurrent_access_of_local_capture,none,
       "%select{mutation of|reference to}0 captured %kind1 in "
       "concurrently-executing code",
@@ -5796,6 +5796,10 @@ ERROR(nonisolated_wrapped_property,none,
       "'nonisolated' is not supported on properties with property wrappers",
       ())
 
+ERROR(non_sendable_from_deinit,none,
+        "cannot access %1 %2 with a non-sendable type %0 from non-isolated deinit",
+        (Type, DescriptiveDeclKind, DeclName))
+
 ERROR(actor_instance_property_wrapper,none,
       "%0 property in property wrapper type %1 cannot be isolated to "
       "the actor instance; consider 'nonisolated'",

include/swift/Sema/Concurrency.h

@@ -22,8 +22,10 @@
 
 namespace swift {
 
+class DeclContext;
 class SourceFile;
 class NominalTypeDecl;
+class VarDecl;
 
 /// If any of the imports in this source file was @preconcurrency but there were
 /// no diagnostics downgraded or suppressed due to that @preconcurrency, suggest
@@ -35,6 +37,13 @@ void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
 bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
                                     bool applyModuleDefault = true);
 
+/// Diagnose the use of an instance property of non-sendable type from an
+/// nonisolated deinitializer within an actor-isolated type.
+///
+/// \returns true iff a diagnostic was emitted for this reference.
+bool diagnoseNonSendableFromDeinit(
+    SourceLoc refLoc, VarDecl *var, DeclContext *dc);
+
 } // namespace swift
 
 #endif

lib/SILOptimizer/Mandatory/FlowIsolation.cpp

@@ -16,6 +16,7 @@
 #include "swift/AST/Expr.h"
 #include "swift/AST/ActorIsolation.h"
 #include "swift/AST/DiagnosticsSIL.h"
+#include "swift/Sema/Concurrency.h"
 #include "swift/SIL/ApplySite.h"
 #include "swift/SIL/BitDataflow.h"
 #include "swift/SIL/BasicBlockBits.h"
@@ -528,31 +529,17 @@ static bool onlyDeinitAccess(RefElementAddrInst *inst) {
 /// is happening in a deinit that uses flow-isolation.
 /// \returns true iff a diagnostic was emitted for this reference.
 static bool diagnoseNonSendableFromDeinit(RefElementAddrInst *inst) {
-  VarDecl *var = inst->getField();
-  Type ty = var->getTypeInContext();
-  DeclContext *dc = inst->getFunction()->getDeclContext();
+  auto dc = inst->getFunction()->getDeclContext();
 
-// FIXME: we should emit diagnostics in other modes using:
-//
-//  if (!shouldDiagnoseExistingDataRaces(dc))
-//    return false;
-//
-// but until we decide how we want to handle isolated state from deinits,
-// we're going to limit the noise to complete mode for now.
+  // For historical reasons, only diagnose this issue in strict mode.
   if (dc->getASTContext().LangOpts.StrictConcurrencyLevel
-      != StrictConcurrency::Complete)
-      return false;
-
-  if (ty->isSendableType())
+        != StrictConcurrency::Complete)
     return false;
 
-  auto &diag = var->getASTContext().Diags;
-
-  diag.diagnose(inst->getLoc().getSourceLoc(), diag::non_sendable_from_deinit,
-                ty, var->getDescriptiveKind(), var->getName())
-      .warnUntilSwiftVersion(6);
-
-  return true;
+  return swift::diagnoseNonSendableFromDeinit(
+      inst->getLoc().getSourceLoc(),
+      inst->getField(),
+      dc);
 }
 
 class OperandWorklist {

lib/Sema/TypeCheckAttr.cpp

@@ -6938,10 +6938,13 @@ void AttributeChecker::visitSendableAttr(SendableAttr *attr) {
   // Prevent Sendable Attr from being added to methods of non-sendable types
   if (auto *funcDecl = dyn_cast<AbstractFunctionDecl>(D)) {
     if (auto selfDecl = funcDecl->getImplicitSelfDecl()) {
-      if (!selfDecl->getTypeInContext()->isSendableType()) {
-        diagnose(attr->getLocation(), diag::nonsendable_instance_method)
-        .warnUntilSwiftVersion(6);
-      }
+      diagnoseIfAnyNonSendableTypes(
+          selfDecl->getTypeInContext(),
+          SendableCheckContext(funcDecl),
+          Type(),
+          SourceLoc(),
+          attr->getLocation(),
+          diag::nonsendable_instance_method);
     }
   }
 }
@@ -6965,13 +6968,16 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) {
 
       // 'nonisolated' without '(unsafe)' is not allowed on non-Sendable variables.
       auto type = var->getTypeInContext();
-      if (!attr->isUnsafe() && !type->hasError() &&
-          !type->isSendableType()) {
-        Ctx.Diags.diagnose(attr->getLocation(),
-                           diag::nonisolated_non_sendable,
-                           type)
-          .warnUntilSwiftVersion(6);
-        return;
+      if (!attr->isUnsafe() && !type->hasError()) {
+        bool diagnosed = diagnoseIfAnyNonSendableTypes(
+            type,
+            SendableCheckContext(dc),
+            Type(),
+            SourceLoc(),
+            attr->getLocation(),
+            diag::nonisolated_non_sendable);
+        if (diagnosed)
+          return;
       }
 
       if (auto nominal = dyn_cast<NominalTypeDecl>(dc)) {

lib/Sema/TypeCheckConcurrency.cpp

@@ -511,7 +511,6 @@ static bool varIsSafeAcrossActors(const ModuleDecl *fromModule,
     return true;
 
   if (!var->isLet()) {
-    ASTContext &ctx = var->getASTContext();
     // A mutable storage of a value type accessed from within the module is
     // okay.
     if (dyn_cast_or_null<StructDecl>(var->getDeclContext()->getAsDecl()) &&
@@ -841,16 +840,25 @@ SendableCheckContext::preconcurrencyBehavior(Decl *decl) const {
     return std::nullopt;
 
   if (auto *nominal = dyn_cast<NominalTypeDecl>(decl)) {
-    // Determine whether this nominal type is visible via a @preconcurrency
-    // import.
-    auto import = nominal->findImport(fromDC);
-    auto sourceFile = fromDC->getParentSourceFile();
+    ModuleDecl *importedModule = nullptr;
+    if (nominal->getAttrs().hasAttribute<PreconcurrencyAttr>()) {
+      // If the declaration itself has the @preconcurrency attribute,
+      // respect it.
+      importedModule = nominal->getParentModule();
+    } else {
+      // Determine whether this nominal type is visible via a @preconcurrency
+      // import.
+      auto import = nominal->findImport(fromDC);
+      auto sourceFile = fromDC->getParentSourceFile();
 
-    if (!import || !import->options.contains(ImportFlags::Preconcurrency))
-      return std::nullopt;
+      if (!import || !import->options.contains(ImportFlags::Preconcurrency))
+        return std::nullopt;
 
-    if (sourceFile)
-      sourceFile->setImportUsedPreconcurrency(*import);
+      if (sourceFile)
+        sourceFile->setImportUsedPreconcurrency(*import);
+
+      importedModule = import->module.importedModule;
+    }
 
     // When the type is explicitly non-Sendable, @preconcurrency imports
     // downgrade the diagnostic to a warning in Swift 6.
@@ -859,7 +867,7 @@ SendableCheckContext::preconcurrencyBehavior(Decl *decl) const {
 
     // When the type is implicitly non-Sendable, `@preconcurrency` suppresses
     // diagnostics until the imported module enables Swift 6.
-    return import->module.importedModule->isConcurrencyChecked()
+    return importedModule->isConcurrencyChecked()
         ? DiagnosticBehavior::Warning
         : DiagnosticBehavior::Ignore;
   }
@@ -5709,7 +5717,7 @@ static bool checkSendableInstanceStorage(
           /*inDerivedConformance*/Type(), element->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
             auto preconcurrency =
-                context.preconcurrencyBehavior(elementType->getAnyNominal());
+                context.preconcurrencyBehavior(type->getAnyNominal());
             if (isImplicitSendableCheck(check)) {
               // If this is for an externally-visible conformance, fail.
               if (check == SendableCheck::ImplicitForExternallyVisible) {
@@ -5720,7 +5728,7 @@ static bool checkSendableInstanceStorage(
               // If we are to ignore this diagnostic, just continue.
               if (behavior == DiagnosticBehavior::Ignore ||
                   preconcurrency == DiagnosticBehavior::Ignore)
-                return false;
+                return true;
 
               invalid = true;
               return true;
@@ -6831,3 +6839,14 @@ ActorReferenceResult ActorReferenceResult::forReference(
 
   return forEntersActor(declIsolation, options);
 }
+
+bool swift::diagnoseNonSendableFromDeinit(
+    SourceLoc refLoc, VarDecl *var, DeclContext *dc) {
+  return diagnoseIfAnyNonSendableTypes(var->getTypeInContext(),
+                                SendableCheckContext(dc),
+                                Type(),
+                                SourceLoc(),
+                                refLoc,
+                                diag::non_sendable_from_deinit,
+                                var->getDescriptiveKind(), var->getName());
+}

lib/Sema/TypeCheckConcurrency.h

@@ -441,7 +441,7 @@ bool diagnoseNonSendableTypes(
       type, fromContext, derivedConformance, typeLoc,
       [&](Type specificType, DiagnosticBehavior behavior) {
         auto preconcurrency =
-            fromContext.preconcurrencyBehavior(type->getAnyNominal());
+          fromContext.preconcurrencyBehavior(specificType->getAnyNominal());
 
         ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
             .limitBehaviorUntilSwiftVersion(behavior, 6)
@@ -477,7 +477,7 @@ bool diagnoseIfAnyNonSendableTypes(
       type, fromContext, derivedConformance, typeLoc,
       [&](Type specificType, DiagnosticBehavior behavior) {
         auto preconcurrency =
-            fromContext.preconcurrencyBehavior(type->getAnyNominal());
+          fromContext.preconcurrencyBehavior(specificType->getAnyNominal());
 
         if (behavior == DiagnosticBehavior::Ignore ||
             preconcurrency == DiagnosticBehavior::Ignore)

test/Concurrency/concurrent_value_checking.swift

@@ -215,7 +215,7 @@ func testUnsafeSendableInAsync() async {
 // ----------------------------------------------------------------------
 // Sendable restriction on key paths.
 // ----------------------------------------------------------------------
-class NC: Hashable { // expected-note 2{{class 'NC' does not conform to the 'Sendable' protocol}}
+class NC: Hashable { // expected-note 3{{class 'NC' does not conform to the 'Sendable' protocol}}
   func hash(into: inout Hasher) { }
   static func==(_: NC, _: NC) -> Bool { true }
 }

test/Concurrency/flow_isolation.swift

@@ -15,7 +15,7 @@ func takeNonSendable(_ ns: NonSendableType) {}
 @available(SwiftStdlib 5.1, *)
 func takeSendable(_ s: SendableType) {}
 
-class NonSendableType {
+class NonSendableType { // expected-note *{{class 'NonSendableType' does not conform to the 'Sendable' protocol}}
   var x: Int = 0
   func f() {}
 }

test/Concurrency/global_variables.swift

@@ -23,7 +23,7 @@ final class TestSendable: Sendable {
   init() {}
 }
 
-final class TestNonsendable { // expected-note 2{{class 'TestNonsendable' does not conform to the 'Sendable' protocol}}
+final class TestNonsendable { // expected-note 3{{class 'TestNonsendable' does not conform to the 'Sendable' protocol}}
   init() {}
 }
 

test/Concurrency/predates_concurrency_import.swift

@@ -16,20 +16,50 @@
 @preconcurrency import OtherActors
 // expected-warning@-1{{'@preconcurrency' attribute on module 'OtherActors' has no effect}}{{1-17=}}
 
+@preconcurrency
+class MyPredatesConcurrencyClass { }
+
+enum EnumWithPredatesConcurrencyValue {
+  case stored(MyPredatesConcurrencyClass)
+}
+
 func acceptSendable<T: Sendable>(_: T) { }
 
 @available(SwiftStdlib 5.1, *)
 func test(
   ss: StrictStruct, ns: NonStrictClass, oma: OtherModuleActor,
-  ssc: SomeSendableClass
+  ssOpt: StrictStruct?, nsOpt: NonStrictClass?,
+  ssc: SomeSendableClass,
+  mpcc: MyPredatesConcurrencyClass
 ) async {
   acceptSendable(ss) // expected-warning{{type 'StrictStruct' does not conform to the 'Sendable' protocol}}
   acceptSendable(ns) // silence issue entirely
+  acceptSendable(ssOpt) // expected-warning{{type 'StrictStruct' does not conform to the 'Sendable' protocol}}
+  acceptSendable(nsOpt) // silence issue entirely
   acceptSendable(oma) // okay
   acceptSendable(ssc) // okay
+  acceptSendable(mpcc)
 }
 
 let nonStrictGlobal = NonStrictClass() // no warning
 
 let strictGlobal = StrictStruct() // expected-warning{{let 'strictGlobal' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
 // expected-note@-1{{isolate 'strictGlobal' to a global actor, or conform 'StrictStruct' to 'Sendable'}}
+
+extension NonStrictClass {
+  @Sendable func f() { }
+}
+
+extension StrictStruct {
+  @Sendable func f() { } // expected-warning{{instance method of non-Sendable type 'StrictStruct' cannot be marked as '@Sendable'}}
+}
+
+
+struct HasStatics {
+  nonisolated static let ns: NonStrictClass = NonStrictClass()
+
+  nonisolated static let ss: StrictStruct = StrictStruct()
+  // expected-warning@-1{{'nonisolated' can not be applied to variable with non-'Sendable' type 'StrictStruct'}}
+  // expected-warning@-2{{static property 'ss' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
+  // expected-note@-3{{isolate 'ss' to a global actor, or conform 'StrictStruct' to 'Sendable'}}
+}

test/Concurrency/predates_concurrency_import_deinit.swift

@@ -0,0 +1,22 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -swift-version 6 %S/Inputs/StrictModule.swift
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
+
+// RUN: %target-swift-frontend  -I %t %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
+
+// REQUIRES: concurrency
+// REQUIRES: asserts
+
+@preconcurrency import NonStrictModule
+@preconcurrency import StrictModule
+
+@available(SwiftStdlib 5.1, *)
+actor ActorWithDeinit {
+  var ns: NonStrictClass = NonStrictClass()
+  var ss: StrictStruct = StrictStruct()
+
+  deinit {
+    print(ns)
+    print(ss) // expected-warning{{cannot access property 'ss' with a non-sendable type 'StrictStruct' from non-isolated deinit}}
+  }
+}

test/Concurrency/predates_concurrency_import_swift6.swift

@@ -2,8 +2,8 @@
 // RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -swift-version 6 %S/Inputs/StrictModule.swift
 // RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
 
-// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -parse-as-library
-// RUN: %target-swift-frontend -swift-version 6 -I %t %s %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation -parse-as-library
+// RUN: %target-swift-frontend -swift-version 6 -I %t %s -emit-sil -o /dev/null -verify -parse-as-library
+// RUN: %target-swift-frontend -swift-version 6 -I %t %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation -parse-as-library
 
 @preconcurrency import NonStrictModule
 @preconcurrency import StrictModule
@@ -19,3 +19,11 @@ func test(ss: StrictStruct, ns: NonStrictClass) {
 let nonStrictGlobal = NonStrictClass()
 let strictGlobal = StrictStruct() // expected-warning{{let 'strictGlobal' is not concurrency-safe because non-'Sendable' type 'StrictStruct' may have shared mutable state}}
 // expected-note@-1{{isolate 'strictGlobal' to a global actor, or conform 'StrictStruct' to 'Sendable'}}
+
+extension NonStrictClass {
+  @Sendable func f() { }
+}
+
+extension StrictStruct {
+  @Sendable func f() { } // expected-warning{{instance method of non-Sendable type 'StrictStruct' cannot be marked as '@Sendable'}}
+}