Merge pull request #42255 from beccadax/witness-accounts-are-only-approximate-2

 Make @preconcurrency import hide sendable variance

Author: beccadax

lib/Sema/TypeCheckConcurrency.cpp

@@ -650,10 +650,17 @@ DiagnosticBehavior SendableCheckContext::defaultDiagnosticBehavior() const {
   return defaultSendableDiagnosticBehavior(fromDC->getASTContext().LangOpts);
 }
 
-/// Determine whether the given nominal type that is within the current module
-/// has an explicit Sendable.
-static bool hasExplicitSendableConformance(NominalTypeDecl *nominal) {
+/// Determine whether the given nominal type has an explicit Sendable
+/// conformance (regardless of its availability).
+static bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
+                                           bool applyModuleDefault = true) {
   ASTContext &ctx = nominal->getASTContext();
+  auto nominalModule = nominal->getParentModule();
+
+  // In a concurrency-checked module, a missing conformance is equivalent to
+  // an explicitly unavailable one. If we want to apply this rule, do so now.
+  if (applyModuleDefault && nominalModule->isConcurrencyChecked())
+    return true;
 
   // Look for any conformance to `Sendable`.
   auto proto = ctx.getProtocol(KnownProtocolKind::Sendable);
@@ -662,7 +669,7 @@ static bool hasExplicitSendableConformance(NominalTypeDecl *nominal) {
 
   // Look for a conformance. If it's present and not (directly) missing,
   // we're done.
-  auto conformance = nominal->getParentModule()->lookupConformance(
+  auto conformance = nominalModule->lookupConformance(
       nominal->getDeclaredInterfaceType(), proto, /*allowMissing=*/true);
   return conformance &&
       !(isa<BuiltinProtocolConformance>(conformance.getConcrete()) &&
@@ -706,18 +713,13 @@ static Optional<AttributedImport<ImportedModule>> findImportFor(
 /// nominal type.
 DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
     NominalTypeDecl *nominal) const {
-  // Determine whether the type was explicitly non-Sendable.
-  auto nominalModule = nominal->getParentModule();
-  bool isExplicitlyNonSendable = nominalModule->isConcurrencyChecked() ||
-      hasExplicitSendableConformance(nominal);
-
   // Determine whether this nominal type is visible via a @preconcurrency
   // import.
   auto import = findImportFor(nominal, fromDC);
+  auto sourceFile = fromDC->getParentSourceFile();
 
   // When the type is explicitly non-Sendable...
-  auto sourceFile = fromDC->getParentSourceFile();
-  if (isExplicitlyNonSendable) {
+  if (hasExplicitSendableConformance(nominal)) {
     // @preconcurrency imports downgrade the diagnostic to a warning in Swift 6,
     if (import && import->options.contains(ImportFlags::Preconcurrency)) {
       if (sourceFile)
@@ -737,7 +739,7 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
     if (sourceFile)
       sourceFile->setImportUsedPreconcurrency(*import);
 
-    return nominalModule->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
+    return nominal->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
         ? DiagnosticBehavior::Warning
         : DiagnosticBehavior::Ignore;
   }
@@ -755,48 +757,31 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
   return defaultBehavior;
 }
 
-/// Produce a diagnostic for a single instance of a non-Sendable type where
-/// a Sendable type is required.
-static bool diagnoseSingleNonSendableType(
-    Type type, SendableCheckContext fromContext, SourceLoc loc,
-    llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
-
+bool swift::diagnoseSendabilityErrorBasedOn(
+    NominalTypeDecl *nominal, SendableCheckContext fromContext,
+    llvm::function_ref<bool(DiagnosticBehavior)> diagnose) {
   auto behavior = DiagnosticBehavior::Unspecified;
 
-  auto module = fromContext.fromDC->getParentModule();
-  ASTContext &ctx = module->getASTContext();
-  auto nominal = type->getAnyNominal();
   if (nominal) {
     behavior = fromContext.diagnosticBehavior(nominal);
   } else {
     behavior = fromContext.defaultDiagnosticBehavior();
   }
 
-  bool wasSuppressed = diagnose(type, behavior);
-
-  if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
-    // Don't emit any other diagnostics.
-  } else if (type->is<FunctionType>()) {
-    ctx.Diags.diagnose(loc, diag::nonsendable_function_type);
-  } else if (nominal && nominal->getParentModule() == module) {
-    // If the nominal type is in the current module, suggest adding
-    // `Sendable` if it might make sense. Otherwise, just complain.
-    if (isa<StructDecl>(nominal) || isa<EnumDecl>(nominal)) {
-      auto note = nominal->diagnose(
-          diag::add_nominal_sendable_conformance,
-          nominal->getDescriptiveKind(), nominal->getName());
-      addSendableFixIt(nominal, note, /*unchecked=*/false);
-    } else {
-      nominal->diagnose(
-          diag::non_sendable_nominal, nominal->getDescriptiveKind(),
-          nominal->getName());
-    }
-  } else if (nominal) {
-    // Note which nominal type does not conform to `Sendable`.
-    nominal->diagnose(
-        diag::non_sendable_nominal, nominal->getDescriptiveKind(),
-        nominal->getName());
+  bool wasSuppressed = diagnose(behavior);
+
+  bool emittedDiagnostics =
+      behavior != DiagnosticBehavior::Ignore && !wasSuppressed;
+
+  // When the type is explicitly Sendable *or* explicitly non-Sendable, we
+  // assume it has been audited and `@preconcurrency` is not recommended even
+  // though it would actually affect the diagnostic.
+  bool nominalIsImportedAndHasImplicitSendability =
+      nominal &&
+      nominal->getParentModule() != fromContext.fromDC->getParentModule() &&
+      !hasExplicitSendableConformance(nominal);
 
+  if (emittedDiagnostics && nominalIsImportedAndHasImplicitSendability) {
     // This type was imported from another module; try to find the
     // corresponding import.
     Optional<AttributedImport<swift::ImportedModule>> import;
@@ -813,6 +798,8 @@ static bool diagnoseSingleNonSendableType(
         import->importLoc.isValid() && sourceFile &&
         !sourceFile->hasImportUsedPreconcurrency(*import)) {
       SourceLoc importLoc = import->importLoc;
+      ASTContext &ctx = nominal->getASTContext();
+
       ctx.Diags.diagnose(
           importLoc, diag::add_predates_concurrency_import,
           ctx.LangOpts.isSwiftVersionAtLeast(6),
@@ -826,6 +813,51 @@ static bool diagnoseSingleNonSendableType(
   return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
 }
 
+/// Produce a diagnostic for a single instance of a non-Sendable type where
+/// a Sendable type is required.
+static bool diagnoseSingleNonSendableType(
+    Type type, SendableCheckContext fromContext, SourceLoc loc,
+    llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
+
+  auto module = fromContext.fromDC->getParentModule();
+  auto nominal = type->getAnyNominal();
+
+  return diagnoseSendabilityErrorBasedOn(nominal, fromContext,
+                                         [&](DiagnosticBehavior behavior) {
+    bool wasSuppressed = diagnose(type, behavior);
+
+    // Don't emit the following notes if we didn't have any diagnostics to
+    // attach them to.
+    if (wasSuppressed || behavior == DiagnosticBehavior::Ignore)
+      return true;
+
+    if (type->is<FunctionType>()) {
+      module->getASTContext().Diags
+          .diagnose(loc, diag::nonsendable_function_type);
+    } else if (nominal && nominal->getParentModule() == module) {
+      // If the nominal type is in the current module, suggest adding
+      // `Sendable` if it might make sense. Otherwise, just complain.
+      if (isa<StructDecl>(nominal) || isa<EnumDecl>(nominal)) {
+        auto note = nominal->diagnose(
+            diag::add_nominal_sendable_conformance,
+            nominal->getDescriptiveKind(), nominal->getName());
+        addSendableFixIt(nominal, note, /*unchecked=*/false);
+      } else {
+        nominal->diagnose(
+            diag::non_sendable_nominal, nominal->getDescriptiveKind(),
+            nominal->getName());
+      }
+    } else if (nominal) {
+      // Note which nominal type does not conform to `Sendable`.
+      nominal->diagnose(
+          diag::non_sendable_nominal, nominal->getDescriptiveKind(),
+          nominal->getName());
+    }
+
+    return false;
+  });
+}
+
 bool swift::diagnoseNonSendableTypes(
     Type type, SendableCheckContext fromContext, SourceLoc loc,
     llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
@@ -1036,7 +1068,7 @@ void swift::diagnoseMissingExplicitSendable(NominalTypeDecl *nominal) {
     return;
 
   // If the conformance is explicitly stated, do nothing.
-  if (hasExplicitSendableConformance(nominal))
+  if (hasExplicitSendableConformance(nominal, /*applyModuleDefault=*/false))
     return;
 
   // Diagnose it.

lib/Sema/TypeCheckConcurrency.h

@@ -353,9 +353,10 @@ struct SendableCheckContext {
 ///
 /// \param diagnose Emit a diagnostic indicating that the current type
 /// is non-Sendable, with the suggested behavior limitation. Returns \c true
-/// if an error was produced.
+/// if it did not emit any diagnostics.
 ///
-/// \returns \c true if any diagnostics were produced, \c false otherwise.
+/// \returns \c true if any errors were produced, \c false if no diagnostics or
+/// only warnings and notes were produced.
 bool diagnoseNonSendableTypes(
     Type type, SendableCheckContext fromContext, SourceLoc loc,
     llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose);
@@ -370,7 +371,8 @@ namespace detail {
 /// Diagnose any non-Sendable types that occur within the given type, using
 /// the given diagnostic.
 ///
-/// \returns \c true if any errors were produced, \c false otherwise.
+/// \returns \c true if any errors were produced, \c false if no diagnostics or
+/// only warnings and notes were produced.
 template<typename ...DiagArgs>
 bool diagnoseNonSendableTypes(
     Type type, SendableCheckContext fromContext, SourceLoc loc,
@@ -389,6 +391,26 @@ bool diagnoseNonSendableTypes(
   });
 }
 
+/// Diagnose this sendability error with behavior based on the import of
+/// \p nominal . For instance, depending on how \p nominal is imported into
+/// \p fromContext , the diagnostic behavior limitation may be lower or the
+/// compiler might emit a fix-it adding \c \@preconcurrency to the \c import .
+///
+/// \param nominal The declaration whose import we should check, or \c nullptr
+/// to get behavior for a non-nominal type.
+///
+/// \param fromContext The context where the error will be emitted.
+///
+/// \param diagnose Emit a diagnostic indicating a sendability problem,
+/// with the suggested behavior limitation. Returns \c true
+/// if it did \em not emit any diagnostics.
+///
+/// \returns \c true if any errors were produced, \c false if no diagnostics or
+/// only warnings and notes were produced.
+bool diagnoseSendabilityErrorBasedOn(
+    NominalTypeDecl *nominal, SendableCheckContext fromContext,
+    llvm::function_ref<bool(DiagnosticBehavior)> diagnose);
+
 /// Given a set of custom attributes, pick out the global actor attributes
 /// and perform any necessary resolution and diagnostics, returning the
 /// global actor attribute and type it refers to (or \c None).

lib/Sema/TypeCheckDeclOverride.cpp

@@ -15,6 +15,7 @@
 //===----------------------------------------------------------------------===//
 #include "MiscDiagnostics.h"
 #include "TypeCheckAvailability.h"
+#include "TypeCheckConcurrency.h"
 #include "TypeCheckDecl.h"
 #include "TypeCheckObjC.h"
 #include "TypeChecker.h"
@@ -548,12 +549,20 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
     diags.diagnose(decl, diag::override_multiple_decls_base,
                    decl->getName());
     break;
-  case OverrideCheckingAttempt::MismatchedSendability:
-    // FIXME: suppress if any matches brought in via @preconcurrency import?
-    diags.diagnose(decl, diag::override_sendability_mismatch,
-                   decl->getName())
-      .warnUntilSwiftVersion(6);
+  case OverrideCheckingAttempt::MismatchedSendability: {
+    SendableCheckContext fromContext(decl->getDeclContext(),
+                                     SendableCheck::Explicit);
+    auto baseDeclClass =
+        decl->getOverriddenDecl()->getDeclContext()->getSelfClassDecl();
+
+    diagnoseSendabilityErrorBasedOn(baseDeclClass, fromContext,
+                                    [&](DiagnosticBehavior limit) {
+      diags.diagnose(decl, diag::override_sendability_mismatch, decl->getName())
+          .limitBehavior(limit);
+      return false;
+    });
     break;
+  }
   case OverrideCheckingAttempt::BaseName:
     diags.diagnose(decl, diag::override_multiple_decls_arg_mismatch,
                    decl->getName());
@@ -1288,11 +1297,19 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
     return true;
 
   if (attempt == OverrideCheckingAttempt::MismatchedSendability) {
-    // FIXME: suppress if any matches brought in via @preconcurrency import?
-    diags.diagnose(decl, diag::override_sendability_mismatch,
-                   decl->getName())
-      .warnUntilSwiftVersion(6);
-    diags.diagnose(baseDecl, diag::overridden_here);
+    SendableCheckContext fromContext(decl->getDeclContext(),
+                                     SendableCheck::Explicit);
+    auto baseDeclClass = baseDecl->getDeclContext()->getSelfClassDecl();
+
+    diagnoseSendabilityErrorBasedOn(baseDeclClass, fromContext,
+                                    [&](DiagnosticBehavior limit) {
+      diags.diagnose(decl, diag::override_sendability_mismatch,
+                     decl->getName())
+        .limitBehavior(limit);
+      diags.diagnose(baseDecl, diag::overridden_here)
+        .limitBehavior(limit);
+      return false;
+    });
   }
   // Catch-all to make sure we don't silently accept something we shouldn't.
   else if (attempt != OverrideCheckingAttempt::PerfectMatch) {

lib/Sema/TypeCheckProtocol.cpp

@@ -4154,19 +4154,32 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
                          requirement->getName());
         });
     }
-    if (best.Kind == MatchKind::RequiresNonSendable)
-      diagnoseOrDefer(requirement, getASTContext().isSwiftVersionAtLeast(6),
-                      [this, requirement, witness]
-                          (NormalProtocolConformance *conformance) {
-        auto &diags = DC->getASTContext().Diags;
+    if (best.Kind == MatchKind::RequiresNonSendable) {
+      SendableCheckContext sendFrom(witness->getDeclContext(),
+                                    SendableCheck::Explicit);
 
-        diags.diagnose(getLocForDiagnosingWitness(conformance, witness),
-                       diag::witness_not_as_sendable,
-                       witness->getDescriptiveKind(), witness->getName(),
-                       conformance->getProtocol()->getName())
-            .warnUntilSwiftVersion(6);
-        diags.diagnose(requirement, diag::less_sendable_reqt_here);
-      });
+      auto behavior = sendFrom.diagnosticBehavior(Conformance->getProtocol());
+      if (behavior != DiagnosticBehavior::Ignore) {
+        bool isError = behavior < DiagnosticBehavior::Warning;
+        
+        diagnoseOrDefer(requirement, isError,
+                        [this, requirement, witness, sendFrom](
+                          NormalProtocolConformance *conformance) {
+          diagnoseSendabilityErrorBasedOn(conformance->getProtocol(), sendFrom,
+                                          [&](DiagnosticBehavior limit) {
+            auto &diags = DC->getASTContext().Diags;
+            diags.diagnose(getLocForDiagnosingWitness(conformance, witness),
+                           diag::witness_not_as_sendable,
+                           witness->getDescriptiveKind(), witness->getName(),
+                           conformance->getProtocol()->getName())
+                .limitBehavior(limit);
+            diags.diagnose(requirement, diag::less_sendable_reqt_here)
+                .limitBehavior(limit);
+            return false;
+          });
+        });
+      }
+    }
 
     auto check = checkWitness(requirement, best);
 

test/Concurrency/Inputs/NonStrictModule.swift

@@ -1,3 +1,11 @@
-public class NonStrictClass { }
-
 public struct NonStrictStruct { }
+
+open class NonStrictClass {
+  open func send(_ body: @Sendable () -> Void) {}
+  open func dontSend(_ body: () -> Void) {}
+}
+
+public protocol NonStrictProtocol {
+  func send(_ body: @Sendable () -> Void)
+  func dontSend(_ body: () -> Void)
+}

test/Concurrency/Inputs/StrictModule.swift

@@ -1 +1,11 @@
 public struct StrictStruct: Hashable { }
+
+open class StrictClass {
+  open func send(_ body: @Sendable () -> Void) {}
+  open func dontSend(_ body: () -> Void) {}
+}
+
+public protocol StrictProtocol {
+  func send(_ body: @Sendable () -> Void)
+  func dontSend(_ body: () -> Void)
+}