[ConformanceLookup] Just kidding, the compiler needs to prefer available

Sendable conformances for source compatibility.

If conformance lookup always prefers the conformance from the defining module,
libraries introducing unavailable Sendable conformances can break source in
clients that declare retroactive Sendable conformances. Instead, still prefer
the available conformance, and always diagnose the client conformance as
redundant (as a warning). Then, when the retroactive conformance is removed,
the errors will surface, so the programmer has to take explicit action to
experience the source break.

(cherry picked from commit b139770)

Author: hborla

lib/AST/ConformanceLookupTable.cpp

@@ -68,9 +68,18 @@ void ConformanceLookupTable::ConformanceEntry::markSupersededBy(
   SupersededBy = entry;
 
   if (diagnose) {
+    // If an unavailable Sendable conformance is superseded by a
+    // retroactive one in the client, we need to record this error
+    // at the client decl context.
+    auto *dc = getDeclContext();
+    if (getProtocol()->isMarkerProtocol() && isFixed() &&
+        !entry->isFixed()) {
+      dc = entry->getDeclContext();
+    }
+
     // Record the problem in the conformance table. We'll
     // diagnose these in semantic analysis.
-    table.AllSupersededDiagnostics[getDeclContext()].push_back(this);
+    table.AllSupersededDiagnostics[dc].push_back(this);
   }
 }
 
@@ -596,21 +605,22 @@ 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()) {
+    // Diagnose conflicting marker protocol conformances that differ in
+    // un-availability.
+    diagnoseSuperseded = lhs->getProtocol()->isMarkerProtocol();
+
+    return (lhs->getDeclContext()->isAlwaysAvailableConformanceContext()
+            ? Ordering::Before
+            : Ordering::After);
   }
 
   // If one entry is fixed and the other is not, we have our answer.
@@ -1128,9 +1138,17 @@ void ConformanceLookupTable::lookupConformances(
   if (diagnostics) {
     auto knownDiags = AllSupersededDiagnostics.find(dc);
     if (knownDiags != AllSupersededDiagnostics.end()) {
-      for (const auto *entry : knownDiags->second) {
+      for (auto *entry : knownDiags->second) {
         ConformanceEntry *supersededBy = entry->getSupersededBy();
 
+        // Diagnose the client conformance as superseded.
+        auto *definingModule = nominal->getParentModule();
+        if (entry->getDeclContext()->getParentModule() == definingModule &&
+            supersededBy->getDeclContext()->getParentModule() != definingModule) {
+          supersededBy = entry;
+          entry = entry->getSupersededBy();
+        }
+
         diagnostics->push_back({entry->getProtocol(), 
                                 entry->getDeclaredLoc(),
                                 entry->getKind(),

test/Concurrency/Inputs/SendableConformances.swift

@@ -5,3 +5,5 @@ public class NonSendableClass {}
 extension NonSendableClass: @unchecked Sendable {}
 
 public struct SendableStruct: Sendable {}
+
+public struct AnotherSendableStruct: Sendable {}

test/Concurrency/redundant_sendable_conformance.swift

@@ -13,7 +13,12 @@ extension NonSendableClass: @retroactive @unchecked Sendable {}
 // expected-warning@-1 {{conformance of 'NonSendableClass' to protocol 'Sendable' was already stated in the type's module 'SendableConformances'}}
 
 typealias CheckNonSendableClass = RequireSendable<NonSendableClass>
-// expected-error@-1 {{conformance of 'NonSendableClass' to 'Sendable' is unavailable}}
 
 extension SendableStruct: @retroactive @unchecked Sendable {}
 // expected-warning@-1 {{conformance of 'SendableStruct' to protocol 'Sendable' was already stated in the type's module 'SendableConformances'}}
+
+@available(*, unavailable)
+extension AnotherSendableStruct: @retroactive @unchecked Sendable {}
+// expected-warning@-1 {{conformance of 'AnotherSendableStruct' to protocol 'Sendable' was already stated in the type's module 'SendableConformances'}}
+
+typealias CheckAnotherSendableStruct = RequireSendable<AnotherSendableStruct>