Revert "[ConformanceLookup] Always prefer unavailable Sendable conformances from the"

hborla · hborla · commit 5e3cf7db40a4 · 2024-07-25T08:24:31.000-07:00
This reverts commit 8159d20.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -3129,9 +3129,6 @@ WARNING(witness_deprecated,none,
         "protocol %1%select{|: %2}2",
         (const ValueDecl *, Identifier, StringRef))
 
-WARNING(unavailable_conformance,none,
-        "conformance of %0 to protocol %1 is already unavailable",
-        (Type, Identifier))
 ERROR(redundant_conformance,none,
       "redundant conformance of %0 to protocol %1", (Type, Identifier))
 ERROR(redundant_conformance_conditional,none,
diff --git a/lib/AST/ConformanceLookupTable.cpp b/lib/AST/ConformanceLookupTable.cpp
@@ -596,31 +596,47 @@ ConformanceLookupTable::Ordering ConformanceLookupTable::compareConformances(
     }
   }
 
-  // Unavailable Sendable conformances cannot be replaced by available ones.
-  if (!lhs->getProtocol()->isMarkerProtocol()) {
-    // If only one of the conformances is unconditionally available on the
-    // current deployment target, pick that one.
-    //
-    // FIXME: Conformance lookup should really depend on source location for
-    // this to be 100% correct.
-    // FIXME: When a class and an extension with the same availability declare the
-    // same conformance, this silently takes the class and drops the extension.
-    if (lhs->getDeclContext()->isAlwaysAvailableConformanceContext() !=
-        rhs->getDeclContext()->isAlwaysAvailableConformanceContext()) {
-      return (lhs->getDeclContext()->isAlwaysAvailableConformanceContext()
-              ? Ordering::Before
-              : Ordering::After);
-    }
+  // If only one of the conformances is unconditionally available on the
+  // current deployment target, pick that one.
+  //
+  // FIXME: Conformance lookup should really depend on source location for
+  // this to be 100% correct.
+  // FIXME: When a class and an extension with the same availability declare the
+  // same conformance, this silently takes the class and drops the extension.
+  if (lhs->getDeclContext()->isAlwaysAvailableConformanceContext() !=
+      rhs->getDeclContext()->isAlwaysAvailableConformanceContext()) {
+    return (lhs->getDeclContext()->isAlwaysAvailableConformanceContext()
+            ? Ordering::Before
+            : Ordering::After);
   }
 
   // If one entry is fixed and the other is not, we have our answer.
   if (lhs->isFixed() != rhs->isFixed()) {
+    auto isReplaceableOrMarker = [](ConformanceEntry *entry) -> bool {
+      ConformanceEntryKind kind = entry->getRankingKind();
+      if (isReplaceable(kind))
+        return true;
+
+      // Allow replacement of an explicit conformance to a marker protocol.
+      // (This permits redundant explicit declarations of `Sendable`.)
+      //
+      // FIXME: We need to warn on attempts to make an unavailable Sendable
+      // conformance available, which does not work.
+      //
+      // We probably also want to warn if there is an existing, explicit
+      // conformance, so clients are prompted to remove retroactive unchecked
+      // Sendable conformances when the proper Sendable conformance is added
+      // in the original module.
+      return (kind == ConformanceEntryKind::Explicit
+              && entry->getProtocol()->isMarkerProtocol());
+    };
+
     // If the non-fixed conformance is not replaceable, we have a failure to
     // diagnose.
     // FIXME: We should probably diagnose if they have different constraints.
-    diagnoseSuperseded = (lhs->isFixed() && !isReplaceable(rhs->getRankingKind())) ||
-                         (rhs->isFixed() && !isReplaceable(lhs->getRankingKind()));
-
+    diagnoseSuperseded = (lhs->isFixed() && !isReplaceableOrMarker(rhs)) ||
+                         (rhs->isFixed() && !isReplaceableOrMarker(lhs));
+      
     return lhs->isFixed() ? Ordering::Before : Ordering::After;
   }
 
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -6442,16 +6442,8 @@ ProtocolConformance *swift::deriveImplicitSendableConformance(
 
   // Local function to form the implicit conformance.
   auto formConformance = [&](const DeclAttribute *attrMakingUnavailable)
-        -> ProtocolConformance * {
+        -> NormalProtocolConformance * {
     DeclContext *conformanceDC = nominal;
-
-    // FIXME: @_nonSendable should be a builtin extension macro. This behavior
-    // of explanding the unavailable conformance during implicit Sendable
-    // derivation means that clients can unknowingly ignore unavailable Sendable
-    // Sendable conformances from the original module added via @_nonSendable
-    // because they are not expanded if an explicit conformance is found via
-    // conformance lookup. So, if a retroactive, unchecked Sendable conformance
-    // is written, no redundant conformance warning is emitted.
     if (attrMakingUnavailable) {
       // Conformance availability is currently tied to the declaring extension.
       // FIXME: This is a hack--we should give conformances real availability.
@@ -6479,18 +6471,6 @@ ProtocolConformance *swift::deriveImplicitSendableConformance(
         file->getOrCreateSynthesizedFile().addTopLevelDecl(extension);
 
       conformanceDC = extension;
-
-      // Let the conformance lookup table register the conformance
-      // from the extension. Otherwise, we'll end up with redundant
-      // conformances between the explicit conformance from the extension
-      // and the conformance synthesized below.
-      SmallVector<ProtocolConformance *, 2> conformances;
-      nominal->lookupConformance(proto, conformances);
-      for (auto conformance : conformances) {
-        if (conformance->getDeclContext() == conformanceDC) {
-          return conformance;
-        }
-      }
     }
 
     auto conformance = ctx.getNormalConformance(
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -6263,56 +6263,20 @@ void TypeChecker::checkConformancesInContext(IterableDeclContext *idc) {
     // protocol, just warn; we'll pick up the original conformance.
     auto existingModule = diag.ExistingDC->getParentModule();
     auto extendedNominal = diag.ExistingDC->getSelfNominalTypeDecl();
-    auto definingModule = extendedNominal->getParentModule()->getName();
-    bool conformanceInOrigModule =
-        (existingModule->getName() == definingModule ||
+    if (existingModule != dc->getParentModule() &&
+        (existingModule->getName() ==
+           extendedNominal->getParentModule()->getName() ||
          existingModule == diag.Protocol->getParentModule() ||
-         existingModule->getName().is("CoreGraphics"));
-
-    // Redundant Sendable conformances are always warnings.
-    auto knownProtocol = diag.Protocol->getKnownProtocolKind();
-    bool isSendable = knownProtocol == KnownProtocolKind::Sendable;
-    // Try to find an inherited Sendable conformance if there is one.
-    if (isSendable && !SendableConformance) {
-      SmallVector<ProtocolConformance *, 2> conformances;
-      nominal->lookupConformance(diag.Protocol, conformances);
-      for (auto conformance : conformances) {
-        if (isa<InheritedProtocolConformance>(conformance))
-          SendableConformance = conformance;
-      }
-    }
-
-    if ((existingModule != dc->getParentModule() && conformanceInOrigModule) ||
-        isSendable) {
+         existingModule->getName().is("CoreGraphics"))) {
       // Warn about the conformance.
-      if (isSendable && SendableConformance &&
-          isa<InheritedProtocolConformance>(SendableConformance)) {
-        // Allow re-stated unchecked conformances to Sendable in subclasses
-        // as long as the inherited conformance isn't unavailable.
-        auto *conformance = SendableConformance->getRootConformance();
-        auto *decl = conformance->getDeclContext()->getAsDecl();
-        if (!AvailableAttr::isUnavailable(decl)) {
-          continue;
-        }
-
-        Context.Diags.diagnose(diag.Loc, diag::unavailable_conformance,
-                               nominal->getDeclaredInterfaceType(),
-                               diag.Protocol->getName());
-      } else if (existingModule == dc->getParentModule()) {
-        Context.Diags.diagnose(diag.Loc, diag::redundant_conformance,
-                               nominal->getDeclaredInterfaceType(),
-                               diag.Protocol->getName())
-          .limitBehavior(DiagnosticBehavior::Warning);
-      } else {
-        auto diagID = differentlyConditional
-                          ? diag::redundant_conformance_adhoc_conditional
-                          : diag::redundant_conformance_adhoc;
-        Context.Diags.diagnose(diag.Loc, diagID, dc->getDeclaredInterfaceType(),
-                               diag.Protocol->getName(),
-                               existingModule->getName() ==
-                                   extendedNominal->getParentModule()->getName(),
-                               existingModule->getName());
-      }
+      auto diagID = differentlyConditional
+                        ? diag::redundant_conformance_adhoc_conditional
+                        : diag::redundant_conformance_adhoc;
+      Context.Diags.diagnose(diag.Loc, diagID, dc->getDeclaredInterfaceType(),
+                             diag.Protocol->getName(),
+                             existingModule->getName() ==
+                                 extendedNominal->getParentModule()->getName(),
+                             existingModule->getName());
 
       // Complain about any declarations in this extension whose names match
       // a requirement in that protocol.
diff --git a/test/Concurrency/Inputs/SendableConformances.swift b/test/Concurrency/Inputs/SendableConformances.swift
diff --git a/test/Concurrency/redundant_sendable_conformance.swift b/test/Concurrency/redundant_sendable_conformance.swift
diff --git a/test/Concurrency/sendable_checking.swift b/test/Concurrency/sendable_checking.swift
@@ -113,8 +113,8 @@ struct WrapClass<T: NSClass> {
 
 extension WrapClass: Sendable where T: Sendable { }
 
-// expected-warning@+2 {{conformance of 'SendableSubclass' to protocol 'Sendable' is already unavailable}}
-// expected-note@+1 {{'SendableSubclass' inherits conformance to protocol 'Sendable' from superclass here}}
+// Make sure we don't inherit the unavailable Sendable conformance from
+// our superclass.
 class SendableSubclass: NSClass, @unchecked Sendable { }
 
 @available(SwiftStdlib 5.1, *)
diff --git a/test/Concurrency/sendable_conformance_checking.swift b/test/Concurrency/sendable_conformance_checking.swift
@@ -180,7 +180,7 @@ extension SendableExtSub: @unchecked Sendable {}
 
 // Still want to know about same-class redundancy
 class MultiConformance: @unchecked Sendable {} // expected-note {{'MultiConformance' declares conformance to protocol 'Sendable' here}}
-extension MultiConformance: @unchecked Sendable {} // expected-warning {{redundant conformance of 'MultiConformance' to protocol 'Sendable'}}
+extension MultiConformance: @unchecked Sendable {} // expected-error {{redundant conformance of 'MultiConformance' to protocol 'Sendable'}}
 
 @available(SwiftStdlib 5.1, *)
 actor MyActor {
