[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.

Author: hborla

lib/AST/ConformanceLookupTable.cpp

@@ -69,9 +69,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);
   }
 }
 
@@ -597,21 +606,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.
@@ -1127,9 +1137,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/redundant_sendable_conformance.swift

@@ -13,7 +13,6 @@ 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'}}