Merge pull request swiftlang#79596 from hborla/6.1-revert-unavailable-superclass-sendable-conformance

[6.1]Revert [ConformanceLookup] Don't allow skipping inherited unavailable conformances in favor of explicit available ones.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -3176,9 +3176,6 @@ GROUPED_WARNING(
     "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,

lib/AST/ConformanceLookupTable.cpp

@@ -70,18 +70,9 @@ 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[dc].push_back(this);
+    table.AllSupersededDiagnostics[getDeclContext()].push_back(this);
   }
 }
 
@@ -269,6 +260,14 @@ void ConformanceLookupTable::inheritConformances(ClassDecl *classDecl,
   auto addInheritedConformance = [&](ConformanceEntry *entry) {
     auto protocol = entry->getProtocol();
 
+    // Don't add unavailable conformances.
+    if (auto dc = entry->Source.getDeclContext()) {
+      if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
+        if (AvailableAttr::isUnavailable(ext))
+          return;
+      }
+    }
+
     // Don't add redundant conformances here. This is merely an
     // optimization; resolveConformances() would zap the duplicates
     // anyway.
@@ -616,23 +615,30 @@ ConformanceLookupTable::Ordering ConformanceLookupTable::compareConformances(
   // 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.
   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`.)
+      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;
   }
 
@@ -874,6 +880,8 @@ DeclContext *ConformanceLookupTable::getConformingContext(
           return nullptr;
         auto inheritedConformance = swift::lookupConformance(
             superclassTy, protocol, /*allowMissing=*/false);
+        if (inheritedConformance.hasUnavailableConformance())
+          inheritedConformance = ProtocolConformanceRef::forInvalid();
         if (inheritedConformance)
           return superclassDecl;
       } while ((superclassDecl = superclassDecl->getSuperclassDecl()));
@@ -1138,17 +1146,9 @@ void ConformanceLookupTable::lookupConformances(
   if (diagnostics) {
     auto knownDiags = AllSupersededDiagnostics.find(dc);
     if (knownDiags != AllSupersededDiagnostics.end()) {
-      for (auto *entry : knownDiags->second) {
+      for (const 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(),

lib/AST/ProtocolConformanceRef.cpp

@@ -333,8 +333,7 @@ bool ProtocolConformanceRef::hasUnavailableConformance() const {
 
   // Check whether this conformance is on an unavailable extension.
   auto concrete = getConcrete();
-  auto *dc = concrete->getRootConformance()->getDeclContext();
-  auto ext = dyn_cast<ExtensionDecl>(dc);
+  auto ext = dyn_cast<ExtensionDecl>(concrete->getDeclContext());
   if (ext && AvailableAttr::isUnavailable(ext))
     return true;
 

lib/Sema/TypeCheckConcurrency.cpp

@@ -6709,16 +6709,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.
@@ -6746,18 +6738,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(
@@ -6779,6 +6759,9 @@ ProtocolConformance *swift::deriveImplicitSendableConformance(
       auto inheritedConformance = checkConformance(
           classDecl->mapTypeIntoContext(superclass),
           proto, /*allowMissing=*/false);
+      if (inheritedConformance.hasUnavailableConformance())
+        inheritedConformance = ProtocolConformanceRef::forInvalid();
+
       if (inheritedConformance) {
         inheritedConformance = inheritedConformance
             .mapConformanceOutOfContext();

lib/Sema/TypeCheckProtocol.cpp

@@ -6370,56 +6370,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) ||
-        diag.Protocol->isMarkerProtocol()) {
+         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.

test/Concurrency/Inputs/SendableConformances.swift

@@ -101,24 +101,21 @@ public actor MyActor: MyProto {
 }
 
 // Make sure the generic signature doesn't minimize away Sendable requirements.
-class NSClass { }
-
-@available(*, unavailable)
-extension NSClass: @unchecked Sendable {} // expected-note {{conformance of 'NSClass' to 'Sendable' has been explicitly marked unavailable here}}
+@_nonSendable class NSClass { }
 
 struct WrapClass<T: NSClass> {
   var t: T
 }
 
 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, *)
 func testSubclassing(obj: SendableSubclass) async {
-  acceptCV(obj) // expected-warning {{conformance of 'NSClass' to 'Sendable' is unavailable; this is an error in the Swift 6 language mode}}
+  acceptCV(obj) // okay!
 }
 
 

test/Concurrency/redundant_sendable_conformance.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 {

test/Concurrency/sendable_checking.swift

@@ -142,7 +142,7 @@ struct Burrito<Filling: ~Copyable>: ~Copyable {}
 extension Burrito: Alias {} // expected-error {{conformance to 'Copyable' must be declared in a separate extension}}
 // expected-note@-1 {{'Burrito<Filling>' declares conformance to protocol 'Copyable' here}}
 
-extension Burrito: Copyable & Edible & P {} // expected-warning {{redundant conformance of 'Burrito<Filling>' to protocol 'Copyable'}}
+extension Burrito: Copyable & Edible & P {} // expected-error {{redundant conformance of 'Burrito<Filling>' to protocol 'Copyable'}}
 
 struct Blah<T: ~Copyable>: ~Copyable {}
 extension Blah: P, Q, Copyable, R {} // expected-error {{generic struct 'Blah' required to be 'Copyable' but is marked with '~Copyable'}}