Merge pull request #42273 from beccadax/witness-accounts-are-only-approximate-5.7

Make overloads and witnesses permit @sendable variance + Make @preconcurrency import hide sendable variance

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -2155,6 +2155,13 @@ ERROR(witness_not_accessible_proto,none,
       "requirement in %select{private|fileprivate|internal|public|%error}4 protocol "
       "%5",
       (RequirementKind, DeclName, bool, AccessLevel, AccessLevel, Identifier))
+ERROR(witness_not_as_sendable,none,
+      "sendability of function types in %0 %1 does not match requirement in "
+      "protocol %2",
+      (DescriptiveDeclKind, DeclName, Identifier))
+NOTE(less_sendable_reqt_here,none,
+     "expected sendability to match requirement here",
+     ())
 ERROR(witness_not_accessible_type,none,
       "%select{initializer %1|method %1|%select{|setter for }2property %1"
       "|subscript%select{| setter}2}0 must be as accessible as its enclosing "
@@ -2303,6 +2310,9 @@ NOTE(ambiguous_witnesses_type,none,
      "multiple matching types named %0", (Identifier))
 NOTE(protocol_witness_exact_match,none,
      "candidate exactly matches%0", (StringRef))
+NOTE(protocol_witness_non_sendable,none,
+     "candidate matches except for closure sendability%0%select{; this will be "
+     "an error in Swift 6|}1", (StringRef, bool))
 NOTE(protocol_witness_renamed,none,
      "rename to %0 to satisfy this requirement%1", (DeclName, StringRef))
 NOTE(protocol_witness_kind_conflict,none,
@@ -2602,6 +2612,9 @@ WARNING(generic_param_usable_from_inline_warn,none,
 ERROR(override_multiple_decls_base,none,
       "declaration %0 cannot override more than one superclass declaration",
       (DeclName))
+ERROR(override_sendability_mismatch,none,
+      "declaration %0 has a type with different sendability from any potential "
+      "overrides", (DeclName))
 ERROR(override_multiple_decls_arg_mismatch,none,
       "declaration %0 has different argument labels from any potential "
       "overrides", (DeclName))

include/swift/AST/Types.h

@@ -295,7 +295,9 @@ enum class TypeMatchFlags {
   /// to be non-escaping, but Swift currently does not.
   IgnoreNonEscapingForOptionalFunctionParam = 1 << 4,
   /// Allow compatible opaque archetypes.
-  AllowCompatibleOpaqueTypeArchetypes = 1 << 5
+  AllowCompatibleOpaqueTypeArchetypes = 1 << 5,
+  /// Ignore the @Sendable attributes on functions when matching types.
+  IgnoreFunctionSendability = 1 << 6,
 };
 using TypeMatchOptions = OptionSet<TypeMatchFlags>;
 

lib/AST/Type.cpp

@@ -3167,6 +3167,16 @@ static bool matchesFunctionType(CanAnyFunctionType fn1, CanAnyFunctionType fn2,
           matchMode.contains(TypeMatchFlags::AllowABICompatible))) {
       ext1 = ext1.withThrows(true);
     }
+
+    // Removing '@Sendable' is ABI-compatible because there's nothing wrong with
+    // a function being sendable when it doesn't need to be.
+    if (!ext2.isSendable())
+      ext1 = ext1.withConcurrent(false);
+  }
+
+  if (matchMode.contains(TypeMatchFlags::IgnoreFunctionSendability)) {
+    ext1 = ext1.withConcurrent(false);
+    ext2 = ext2.withConcurrent(false);
   }
 
   // If specified, allow an escaping function parameter to override a

lib/Sema/TypeCheckConcurrency.cpp

@@ -770,10 +770,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);
@@ -782,7 +789,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()) &&
@@ -826,18 +833,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)
@@ -857,7 +859,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;
   }
@@ -875,48 +877,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;
@@ -933,6 +918,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),
@@ -946,6 +933,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) {
@@ -1156,7 +1188,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

@@ -313,9 +313,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);
@@ -330,7 +331,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,
@@ -349,6 +351,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"
@@ -518,6 +519,7 @@ static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
 namespace {
   enum class OverrideCheckingAttempt {
     PerfectMatch,
+    MismatchedSendability,
     MismatchedOptional,
     MismatchedTypes,
     BaseName,
@@ -547,6 +549,20 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
     diags.diagnose(decl, diag::override_multiple_decls_base,
                    decl->getName());
     break;
+  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());
@@ -886,6 +902,7 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
   DeclName name;
   switch (attempt) {
   case OverrideCheckingAttempt::PerfectMatch:
+  case OverrideCheckingAttempt::MismatchedSendability:
   case OverrideCheckingAttempt::MismatchedOptional:
   case OverrideCheckingAttempt::MismatchedTypes:
     name = decl->getName();
@@ -976,6 +993,8 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
       matchMode |= TypeMatchFlags::AllowNonOptionalForIUOParam;
       matchMode |= TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam;
     }
+    if (attempt == OverrideCheckingAttempt::MismatchedSendability)
+      matchMode |= TypeMatchFlags::IgnoreFunctionSendability;
 
     auto declFnTy = getDeclComparisonType()->getAs<AnyFunctionType>();
     auto parentDeclTy = getMemberTypeForComparison(parentDecl, decl);
@@ -1277,8 +1296,23 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
   if (emittedMatchError)
     return true;
 
+  if (attempt == OverrideCheckingAttempt::MismatchedSendability) {
+    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.
-  if (attempt != OverrideCheckingAttempt::PerfectMatch) {
+  else if (attempt != OverrideCheckingAttempt::PerfectMatch) {
     OverrideMatch match{decl, /*isExact=*/false};
     diagnoseGeneralOverrideFailure(decl, match, attempt);
   }
@@ -1374,11 +1408,12 @@ bool swift::checkOverrides(ValueDecl *decl) {
     switch (attempt) {
     case OverrideCheckingAttempt::PerfectMatch:
       break;
-    case OverrideCheckingAttempt::MismatchedOptional:
+    case OverrideCheckingAttempt::MismatchedSendability:
       // Don't keep looking if the user didn't indicate it's an override.
       if (!decl->getAttrs().hasAttribute<OverrideAttr>())
         return false;
       break;
+    case OverrideCheckingAttempt::MismatchedOptional:
     case OverrideCheckingAttempt::MismatchedTypes:
       break;
     case OverrideCheckingAttempt::BaseName: