Merge pull request #71596 from tshortli/multiple-clause-retroactive-conformances

Sema: Only diagnose retroactive conformances once

Author: tshortli

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -1641,6 +1641,7 @@ static void diagnoseRetroactiveConformances(
   // At this point, we know we're extending a type declared outside this module.
   // We better only be conforming it to protocols declared within this module.
   llvm::SmallSetVector<ProtocolDecl *, 8> externalProtocols;
+  llvm::SmallSet<ProtocolDecl *, 8> protocolsWithRetroactiveAttr;
   for (const InheritedEntry &entry : ext->getInherited().getEntries()) {
     if (entry.getType().isNull() || !entry.getTypeRepr()) {
       continue;
@@ -1665,7 +1666,6 @@ static void diagnoseRetroactiveConformances(
       auto conformanceRef = ext->getParentModule()->lookupConformance(
           extendedType, decl);
       if (!conformanceRef.isConcrete()) {
-
         return TypeWalker::Action::Continue;
       }
       auto conformance = conformanceRef.getConcrete();
@@ -1701,6 +1701,10 @@ static void diagnoseRetroactiveConformances(
 
       // If it's marked @retroactive, no need to warn.
       if (entry.isRetroactive()) {
+        // Note that we encountered this protocol through a conformance marked
+        // @retroactive in case multiple clauses cause the protocol to be
+        // inherited.
+        protocolsWithRetroactiveAttr.insert(decl);
         return TypeWalker::Action::Continue;
       }
 
@@ -1712,6 +1716,11 @@ static void diagnoseRetroactiveConformances(
     });
   }
 
+  // Remove protocols that are reachable through a @retroactive conformance.
+  for (auto *proto : protocolsWithRetroactiveAttr) {
+    externalProtocols.remove(proto);
+  }
+
   // If we didn't find any violations, we're done.
   if (externalProtocols.empty()) {
     return;

test/Sema/extension_retroactive_conformances.swift

@@ -6,9 +6,16 @@
 // Define a couple protocols with no requirements that're easy to conform to
 public protocol SampleProtocol1 {}
 public protocol SampleProtocol2 {}
+public protocol SampleProtocol1a: SampleProtocol1 {}
+public protocol SampleProtocol1b: SampleProtocol1 {}
 
 public struct Sample1 {}
 public struct Sample2 {}
+public struct Sample2a {}
+public struct Sample2b {}
+public struct Sample2c {}
+public struct Sample2d {}
+public struct Sample2e {}
 public struct Sample3 {}
 public struct Sample4 {}
 public struct Sample5 {}
@@ -36,6 +43,19 @@ extension Sample2: InheritsSampleProtocol {} // expected-warning {{extension dec
 
 extension SampleAlreadyConforms: InheritsSampleProtocol {} // ok, SampleAlreadyConforms already conforms in the source module
 
+extension Sample2a: @retroactive SampleProtocol1, InheritsSampleProtocol {} // ok, the concrete conformance to SampleProtocol1 has been declared retroactive
+
+extension Sample2b: InheritsSampleProtocol, @retroactive SampleProtocol1 {} // ok, same as above but in the opposite order
+
+// FIXME: It would be better to only suggest marking SampleProtocol1a @retroactive here
+extension Sample2c: SampleProtocol1a {} // expected-warning {{extension declares a conformance of imported type 'Sample2c' to imported protocols 'SampleProtocol1a', 'SampleProtocol1'}}
+// expected-note @-1 {{add '@retroactive' to silence this warning}} {{21-37=@retroactive SampleProtocol1a}} {{1-1=extension Sample2c: @retroactive SampleProtocol1 {\}\n}}
+
+extension Sample2d: @retroactive SampleProtocol1a {} // ok, retroactive conformance to SampleProtocol1a covers conformance to SampleProtocol1
+
+extension Sample2e: SampleProtocol1a, @retroactive SampleProtocol1b {} // expected-warning {{extension declares a conformance of imported type 'Sample2e' to imported protocol 'SampleProtocol1a'}}
+// expected-note @-1 {{add '@retroactive' to silence this warning}} {{21-37=@retroactive SampleProtocol1a}}
+
 extension Sample3: NestedInheritsSampleProtocol {} // expected-warning {{extension declares a conformance of imported type 'Sample3' to imported protocol 'SampleProtocol1'}}
 // expected-note @-1 {{add '@retroactive' to silence this warning}} {{1-1=extension Sample3: @retroactive SampleProtocol1 {\}\n}}