Merge pull request #73212 from gottesmm/move-error

[region-isolation] Suppress non-Sendable region isolation errors appropriately when the type is imported by using @preconcurrency import.

Author: gottesmm

include/swift/Sema/Concurrency.h

@@ -23,12 +23,18 @@
 namespace swift {
 
 class SourceFile;
+class NominalTypeDecl;
 
 /// If any of the imports in this source file was @preconcurrency but there were
 /// no diagnostics downgraded or suppressed due to that @preconcurrency, suggest
 /// that the attribute be removed.
 void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
 
+/// Determine whether the given nominal type has an explicit Sendable
+/// conformance (regardless of its availability).
+bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
+                                    bool applyModuleDefault = true);
+
 } // namespace swift
 
 #endif

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -15,6 +15,8 @@
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/ProtocolConformance.h"
+#include "swift/AST/SourceFile.h"
 #include "swift/AST/Type.h"
 #include "swift/Basic/FrozenMultiMap.h"
 #include "swift/Basic/ImmutablePointerSet.h"
@@ -35,6 +37,7 @@
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/PartitionUtils.h"
 #include "swift/SILOptimizer/Utils/VariableNameUtils.h"
+#include "swift/Sema/Concurrency.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Debug.h"
 
@@ -55,6 +58,33 @@ using Region = PartitionPrimitives::Region;
 //                              MARK: Utilities
 //===----------------------------------------------------------------------===//
 
+static std::optional<DiagnosticBehavior>
+getDiagnosticBehaviorLimitForValue(SILValue value) {
+  auto *nom = value->getType().getNominalOrBoundGenericNominal();
+  if (!nom)
+    return {};
+
+  auto declRef = value->getFunction()->getDeclRef();
+  if (!declRef)
+    return {};
+
+  auto *fromDC = declRef.getInnermostDeclContext();
+  auto attributedImport = nom->findImport(fromDC);
+  if (!attributedImport ||
+      !attributedImport->options.contains(ImportFlags::Preconcurrency))
+    return {};
+
+  if (auto *sourceFile = fromDC->getParentSourceFile())
+    sourceFile->setImportUsedPreconcurrency(*attributedImport);
+
+  if (hasExplicitSendableConformance(nom))
+    return DiagnosticBehavior::Warning;
+
+  return attributedImport->module.importedModule->isConcurrencyChecked()
+             ? DiagnosticBehavior::Warning
+             : DiagnosticBehavior::Ignore;
+}
+
 static Expr *inferArgumentExprFromApplyExpr(ApplyExpr *sourceApply,
                                             FullApplySite fai,
                                             const Operand *op) {
@@ -438,14 +468,19 @@ class UseAfterTransferDiagnosticEmitter {
       emitUnknownPatternError();
   }
 
+  std::optional<DiagnosticBehavior> getBehaviorLimit() const {
+    return getDiagnosticBehaviorLimitForValue(transferOp->get());
+  }
+
   void
   emitNamedIsolationCrossingError(SILLocation loc, Identifier name,
                                   SILIsolationInfo namedValuesIsolationInfo,
                                   ApplyIsolationCrossing isolationCrossing) {
     // Emit the short error.
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
 
     // Then emit the note with greater context.
     SmallString<64> descriptiveKindStr;
@@ -466,7 +501,8 @@ class UseAfterTransferDiagnosticEmitter {
         loc, diag::regionbasedisolation_transfer_yields_race_with_isolation,
         inferredType, isolationCrossing.getCallerIsolation(),
         isolationCrossing.getCalleeIsolation())
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
@@ -475,7 +511,8 @@ class UseAfterTransferDiagnosticEmitter {
     // Emit the short error.
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
 
     // Then emit the note with greater context.
     diagnoseNote(
@@ -493,7 +530,8 @@ class UseAfterTransferDiagnosticEmitter {
         diag::
             regionbasedisolation_transfer_yields_race_stronglytransferred_binding,
         inferredType)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
@@ -502,7 +540,8 @@ class UseAfterTransferDiagnosticEmitter {
     diagnoseError(loc,
                   diag::regionbasedisolation_transfer_yields_race_no_isolation,
                   inferredType)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
@@ -513,7 +552,8 @@ class UseAfterTransferDiagnosticEmitter {
     // Emit the short error.
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
 
     SmallString<64> descriptiveKindStr;
     {
@@ -535,13 +575,15 @@ class UseAfterTransferDiagnosticEmitter {
     diagnoseError(loc, diag::regionbasedisolation_isolated_capture_yields_race,
                   inferredType, isolationCrossing.getCalleeIsolation(),
                   isolationCrossing.getCallerIsolation())
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
   void emitUnknownPatternError() {
     diagnoseError(transferOp->getUser(),
-                  diag::regionbasedisolation_unknown_pattern);
+                  diag::regionbasedisolation_unknown_pattern)
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
 private:
@@ -941,20 +983,26 @@ class TransferNonTransferrableDiagnosticEmitter {
     return info.nonTransferrable.dyn_cast<SILInstruction *>();
   }
 
+  std::optional<DiagnosticBehavior> getBehaviorLimit() const {
+    return getDiagnosticBehaviorLimitForValue(info.transferredOperand->get());
+  }
+
   /// Return the isolation region info for \p getNonTransferrableValue().
   SILIsolationInfo getIsolationRegionInfo() const {
     return info.isolationRegionInfo;
   }
 
   void emitUnknownPatternError() {
     diagnoseError(getOperand()->getUser(),
-                  diag::regionbasedisolation_unknown_pattern);
+                  diag::regionbasedisolation_unknown_pattern)
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitUnknownUse(SILLocation loc) {
     // TODO: This will eventually be an unknown pattern error.
-    diagnoseError(
-        loc, diag::regionbasedisolation_task_or_actor_isolated_transferred);
+    diagnoseError(loc,
+                  diag::regionbasedisolation_task_or_actor_isolated_transferred)
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitFunctionArgumentApply(SILLocation loc, Type type,
@@ -967,7 +1015,8 @@ class TransferNonTransferrableDiagnosticEmitter {
     diagnoseError(loc, diag::regionbasedisolation_arg_transferred,
                   StringRef(descriptiveKindStr), type,
                   crossing.getCalleeIsolation())
-        .highlight(getOperand()->getUser()->getLoc().getSourceRange());
+        .highlight(getOperand()->getUser()->getLoc().getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitNamedFunctionArgumentClosure(SILLocation loc, Identifier name,
@@ -995,13 +1044,15 @@ class TransferNonTransferrableDiagnosticEmitter {
     auto diag =
         diag::regionbasedisolation_arg_passed_to_strongly_transferred_param;
     diagnoseError(loc, diag, descriptiveKindStr, type)
-        .highlight(getOperand()->getUser()->getLoc().getSourceRange());
+        .highlight(getOperand()->getUser()->getLoc().getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitNamedOnlyError(SILLocation loc, Identifier name) {
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(getOperand()->getUser()->getLoc().getSourceRange());
+        .highlight(getOperand()->getUser()->getLoc().getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitNamedIsolation(SILLocation loc, Identifier name,
@@ -1176,7 +1227,7 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
     if (!isolation) {
       // Otherwise, emit a "we don't know error" that tells the user to file a
       // bug.
-      diagnoseError(op->getUser(), diag::regionbasedisolation_unknown_pattern);
+      diagnosticEmitter.emitUnknownPatternError();
       return false;
     }
     assert(isolation && "Expected non-null");

lib/Sema/TypeCheckConcurrency.cpp

@@ -789,8 +789,8 @@ SendableCheckContext::implicitSendableDiagnosticBehavior() const {
 
 /// Determine whether the given nominal type has an explicit Sendable
 /// conformance (regardless of its availability).
-static bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
-                                           bool applyModuleDefault = true) {
+bool swift::hasExplicitSendableConformance(NominalTypeDecl *nominal,
+                                           bool applyModuleDefault) {
   ASTContext &ctx = nominal->getASTContext();
   auto nominalModule = nominal->getParentModule();
 

test/Concurrency/Inputs/transfernonsendable_preconcurrency_checked.swift

@@ -0,0 +1,4 @@
+
+public class NonSendableKlass {
+  public init() {}
+}

test/Concurrency/Inputs/transfernonsendable_preconcurrency_unchecked.swift

@@ -0,0 +1,11 @@
+
+public class NonSendableKlass {
+  public init() {}
+}
+
+public class ExplicitlyNonSendableKlass {
+  public init() {}
+}
+
+@available(*, unavailable)
+extension ExplicitlyNonSendableKlass: Sendable {}