Merge pull request #77463 from slavapestov/fix-rdar139089004

slavapestov · web-flow · commit fcd6985c531d · 2024-11-08T10:45:14.000-05:00
AST: Add workaround for incorrect mangling of conditional conformances with pack requirements
diff --git a/include/swift/AST/GenericSignature.h b/include/swift/AST/GenericSignature.h
@@ -400,7 +400,9 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
   /// checking against global state, if any/all of the types in the requirement
   /// are concrete, not type parameters.
   bool isRequirementSatisfied(
-      Requirement requirement, bool allowMissing = false) const;
+      Requirement requirement,
+      bool allowMissing = false,
+      bool brokenPackBehavior = false) const;
 
   bool isReducedType(Type type) const;
 
diff --git a/lib/AST/ASTMangler.cpp b/lib/AST/ASTMangler.cpp
@@ -1909,19 +1909,32 @@ static bool forEachConditionalConformance(const ProtocolConformance *conformance
   auto *rootConformance = conformance->getRootConformance();
 
   auto subMap = conformance->getSubstitutionMap();
-  for (auto requirement : rootConformance->getConditionalRequirements()) {
-    if (requirement.getKind() != RequirementKind::Conformance)
+
+  auto ext = dyn_cast<ExtensionDecl>(rootConformance->getDeclContext());
+  if (!ext)
+    return false;
+
+  auto typeSig = ext->getExtendedNominal()->getGenericSignature();
+  auto extensionSig = rootConformance->getGenericSignature();
+
+  for (const auto &req : extensionSig.getRequirements()) {
+    // We set brokenPackBehavior to true here to maintain compatibility with
+    // the mangling produced by an old compiler. We could incorrectly return
+    // false from isRequirementSatisfied() here even if the requirement was
+    // satisfied, and then it would show up as a conditional requirement
+    // even though it was already part of the nominal type's generic signature.
+    if (typeSig->isRequirementSatisfied(req,
+                                        /*allowMissing=*/false,
+                                        /*brokenPackBehavior=*/true))
       continue;
-    ProtocolDecl *proto = requirement.getProtocolDecl();
-    auto conformance = subMap.lookupConformance(
-        requirement.getFirstType()->getCanonicalType(), proto);
-    if (conformance.isInvalid()) {
-      // This should only happen when mangling invalid ASTs, but that happens
-      // for indexing purposes.
+
+    if (req.getKind() != RequirementKind::Conformance)
       continue;
-    }
 
-    if (fn(requirement.getFirstType().subst(subMap), conformance))
+    ProtocolDecl *proto = req.getProtocolDecl();
+    auto conformance = subMap.lookupConformance(
+        req.getFirstType()->getCanonicalType(), proto);
+    if (fn(req.getFirstType().subst(subMap), conformance))
       return true;
   }
 
@@ -3473,7 +3486,14 @@ void ASTMangler::gatherGenericSignatureParts(GenericSignature sig,
     genericParams = canSig.getGenericParams();
   } else {
     llvm::erase_if(reqs, [&](Requirement req) {
-      return contextSig->isRequirementSatisfied(req);
+      // We set brokenPackBehavior to true here to maintain compatibility with
+      // the mangling produced by an old compiler. We could incorrectly return
+      // false from isRequirementSatisfied() here even if the requirement was
+      // satisfied, and then it would show up as a conditional requirement
+      // even though it was already part of the nominal type's generic signature.
+      return contextSig->isRequirementSatisfied(req,
+                                                /*allowMissing=*/false,
+                                                /*brokenPackBehavior=*/true);
     });
   }
 }
diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp
@@ -406,10 +406,26 @@ bool GenericSignatureImpl::areReducedTypeParametersEqual(Type type1,
 }
 
 bool GenericSignatureImpl::isRequirementSatisfied(
-    Requirement requirement, bool allowMissing) const {
+    Requirement requirement,
+    bool allowMissing,
+    bool brokenPackBehavior) const {
   if (requirement.getFirstType()->hasTypeParameter()) {
     auto *genericEnv = getGenericEnvironment();
 
+    if (brokenPackBehavior) {
+      // Swift 5.9 shipped with a bug here where this method would return
+      // incorrect results. Maintain the old behavior specifically for two
+      // call sites in the ASTMangler.
+      if ((requirement.getKind() == RequirementKind::SameType ||
+           requirement.getKind() == RequirementKind::Superclass) &&
+          !requirement.getSecondType()->isTypeParameter() &&
+          requirement.getSecondType().findIf([&](Type t) -> bool {
+            return t->is<PackExpansionType>();
+          })) {
+        return false;
+      }
+    }
+
     requirement = requirement.subst(
         QueryInterfaceTypeSubstitutions{genericEnv},
         LookUpConformanceInModule(),
diff --git a/test/IRGen/conditional_pack_requirements.swift b/test/IRGen/conditional_pack_requirements.swift
@@ -0,0 +1,48 @@
+// RUN: %target-swift-frontend -emit-ir %s -target %target-swift-5.9-abi-triple | %FileCheck %s
+
+public protocol P {
+  associatedtype A
+}
+
+public protocol Q {}
+
+public class C<each T> {}
+
+public struct GG1<A: P, each B: P> where A.A == C<repeat (each B).A> {}
+
+extension GG1: Q where A: Q, repeat each B: Q {}
+
+// This mangling is incorrect; the correct mangling is "$s29conditional_pack_requirements3GG1Vyxq_q_Qp_QPGAA1QA2aERzAaER_rlMc"
+// However, we retain the incorrect behavior for ABI compatibility.
+//
+// CHECK-LABEL: @"$s29conditional_pack_requirements3GG1Vyxq_q_Qp_QPGAA1QA2aERzAaER_AA1CCy1AAA1PPQy_q_Qp_QPGAhJRtzrlMc" =
+
+
+public struct GG2<each A: P> {
+  public struct Nested<each B: P> where repeat (each A).A == (each B).A {}
+}
+
+extension GG2.Nested: Q where repeat each A: Q, repeat each B: Q {}
+
+// This mangling is correct.
+// CHECK-LABEL: @"$s29conditional_pack_requirements3GG2V6NestedVyxxQp_QP_qd__qd__Qp_QPGAA1QA2aGRzAaGRd__rlMc" =
+
+
+public struct GG3<A: P, each B: P> where A.A : C<repeat (each B).A> {}
+
+extension GG3: Q where A: Q, repeat each B: Q {}
+
+// This mangling is incorrect; the correct mangling is "$s29conditional_pack_requirements3GG3Vyxq_q_Qp_QPGAA1QA2aERzAaER_rlMc"
+// However, we retain the incorrect behavior for ABI compatibility.
+//
+// CHECK-LABEL: @"$s29conditional_pack_requirements3GG3Vyxq_q_Qp_QPGAA1QA2aERzAaER_AA1CCy1AAA1PPQy_q_Qp_QPGAhJRczrlMc" =
+
+
+public struct GG4<each A: P> {
+  public struct Nested<each B: P> where repeat (each A).A : C<(each B).A> {}
+}
+
+extension GG4.Nested: Q where repeat each A: Q, repeat each B: Q {}
+
+// This mangling is correct.
+// CHECK-LABEL: @"$s29conditional_pack_requirements3GG4V6NestedVyxxQp_QP_qd__qd__Qp_QPGAA1QA2aGRzAaGRd__rlMc" =
diff --git a/test/ModuleInterface/conditional_pack_requirements.swift b/test/ModuleInterface/conditional_pack_requirements.swift
@@ -0,0 +1,48 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-emit-module-interface(%t/conditional_pack_requirements.swiftinterface) %s -target %target-swift-5.9-abi-triple
+// RUN: %FileCheck %s < %t/conditional_pack_requirements.swiftinterface
+
+public protocol P {
+  associatedtype A
+}
+
+public protocol Q {}
+
+public class C<each T> {}
+
+public struct GG1<A: P, each B: P> where A.A == C<repeat (each B).A> {}
+
+extension GG1: Q where A: Q, repeat each B: Q {}
+
+// CHECK-LABEL: public struct GG1<A, each B> where A : conditional_pack_requirements.P, repeat each B : conditional_pack_requirements.P, A.A == conditional_pack_requirements.C<repeat (each B).A> {
+// CHECK-LABEL: extension conditional_pack_requirements.GG1 : conditional_pack_requirements.Q where A : conditional_pack_requirements.Q, repeat each B : conditional_pack_requirements.Q {
+
+
+public struct GG2<each A: P> {
+  public struct Nested<each B: P> where repeat (each A).A == (each B).A {}
+}
+
+extension GG2.Nested: Q where repeat each A: Q, repeat each B: Q {}
+
+// CHECK-LABEL: public struct GG2<each A> where repeat each A : conditional_pack_requirements.P {
+// CHECK-LABEL: public struct Nested<each B> where repeat each B : conditional_pack_requirements.P, repeat (each A).A == (each B).A {
+// CHECK-LABEL: extension conditional_pack_requirements.GG2.Nested : conditional_pack_requirements.Q where repeat each A : conditional_pack_requirements.Q, repeat each B : conditional_pack_requirements.Q {
+
+
+public struct GG3<A: P, each B: P> where A.A : C<repeat (each B).A> {}
+
+extension GG3: Q where A: Q, repeat each B: Q {}
+
+// CHECK-LABEL: public struct GG3<A, each B> where A : conditional_pack_requirements.P, repeat each B : conditional_pack_requirements.P, A.A : conditional_pack_requirements.C<repeat (each B).A> {
+// CHECK-LABEL: extension conditional_pack_requirements.GG3 : conditional_pack_requirements.Q where A : conditional_pack_requirements.Q, repeat each B : conditional_pack_requirements.Q {
+
+
+public struct GG4<each A: P> {
+  public struct Nested<each B: P> where repeat (each A).A : C<(each B).A> {}
+}
+
+extension GG4.Nested: Q where repeat each A: Q, repeat each B: Q {}
+
+// CHECK-LABEL: public struct GG4<each A> where repeat each A : conditional_pack_requirements.P {
+// CHECK-LABEL: public struct Nested<each B> where repeat each B : conditional_pack_requirements.P, repeat (each A).A : conditional_pack_requirements.C<(each B).A> {
+// CHECK-LABEL: extension conditional_pack_requirements.GG4.Nested : conditional_pack_requirements.Q where repeat each A : conditional_pack_requirements.Q, repeat each B : conditional_pack_requirements.Q {
