Skip to content

Commit 69e3aaf

Browse files
committed
AST: Add workaround for incorrect mangling of conditional conformances with pack requirements
I added commit 7eecf97 a while ago to fix a newly-added assertion failure that came up, however this had the inadvertent side effect of changing symbol mangling and ASTPrinter behavior. The problem in both instances was that we would incorrectly return certain requirements as unsatisfied when really they are satisfied. There is nothing to fix in the ASTPrinter, because printing redundant requirements does not change the generic signature of the extension; they are simply dropped. I added a test to exercise the new behavior showing that the requirements are dropped. As for the mangler, the fix introduced an ABI break, because the symbol name of a conformance descriptor includes its conditional requirements, so we must preserve the redundant requirements forever. The IRGen test has some examples of manglings that are incorrect but must be preserved. I'm plumbing down a flag to isRequirementSatified() to preserve compatibility with the old behavior where we would mangle these redundant requirements. No other callers should pass this flag, except for the mangler. Fixes rdar://139089004.
1 parent 8f66bf1 commit 69e3aaf

File tree

5 files changed

+147
-13
lines changed

5 files changed

+147
-13
lines changed

include/swift/AST/GenericSignature.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,9 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
400400
/// checking against global state, if any/all of the types in the requirement
401401
/// are concrete, not type parameters.
402402
bool isRequirementSatisfied(
403-
Requirement requirement, bool allowMissing = false) const;
403+
Requirement requirement,
404+
bool allowMissing = false,
405+
bool brokenPackBehavior = false) const;
404406

405407
bool isReducedType(Type type) const;
406408

lib/AST/ASTMangler.cpp

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,19 +1915,32 @@ static bool forEachConditionalConformance(const ProtocolConformance *conformance
19151915
auto *rootConformance = conformance->getRootConformance();
19161916

19171917
auto subMap = conformance->getSubstitutionMap();
1918-
for (auto requirement : rootConformance->getConditionalRequirements()) {
1919-
if (requirement.getKind() != RequirementKind::Conformance)
1918+
1919+
auto ext = dyn_cast<ExtensionDecl>(rootConformance->getDeclContext());
1920+
if (!ext)
1921+
return false;
1922+
1923+
auto typeSig = ext->getExtendedNominal()->getGenericSignature();
1924+
auto extensionSig = rootConformance->getGenericSignature();
1925+
1926+
for (const auto &req : extensionSig.getRequirements()) {
1927+
// We set brokenPackBehavior to true here to maintain compatibility with
1928+
// the mangling produced by an old compiler. We could incorrectly return
1929+
// false from isRequirementSatisfied() here even if the requirement was
1930+
// satisfied, and then it would show up as a conditional requirement
1931+
// even though it was already part of the nominal type's generic signature.
1932+
if (typeSig->isRequirementSatisfied(req,
1933+
/*allowMissing=*/false,
1934+
/*brokenPackBehavior=*/true))
19201935
continue;
1921-
ProtocolDecl *proto = requirement.getProtocolDecl();
1922-
auto conformance = subMap.lookupConformance(
1923-
requirement.getFirstType()->getCanonicalType(), proto);
1924-
if (conformance.isInvalid()) {
1925-
// This should only happen when mangling invalid ASTs, but that happens
1926-
// for indexing purposes.
1936+
1937+
if (req.getKind() != RequirementKind::Conformance)
19271938
continue;
1928-
}
19291939

1930-
if (fn(requirement.getFirstType().subst(subMap), conformance))
1940+
ProtocolDecl *proto = req.getProtocolDecl();
1941+
auto conformance = subMap.lookupConformance(
1942+
req.getFirstType()->getCanonicalType(), proto);
1943+
if (fn(req.getFirstType().subst(subMap), conformance))
19311944
return true;
19321945
}
19331946

@@ -3479,7 +3492,14 @@ void ASTMangler::gatherGenericSignatureParts(GenericSignature sig,
34793492
genericParams = canSig.getGenericParams();
34803493
} else {
34813494
llvm::erase_if(reqs, [&](Requirement req) {
3482-
return contextSig->isRequirementSatisfied(req);
3495+
// We set brokenPackBehavior to true here to maintain compatibility with
3496+
// the mangling produced by an old compiler. We could incorrectly return
3497+
// false from isRequirementSatisfied() here even if the requirement was
3498+
// satisfied, and then it would show up as a conditional requirement
3499+
// even though it was already part of the nominal type's generic signature.
3500+
return contextSig->isRequirementSatisfied(req,
3501+
/*allowMissing=*/false,
3502+
/*brokenPackBehavior=*/true);
34833503
});
34843504
}
34853505
}

lib/AST/GenericSignature.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,10 +406,26 @@ bool GenericSignatureImpl::areReducedTypeParametersEqual(Type type1,
406406
}
407407

408408
bool GenericSignatureImpl::isRequirementSatisfied(
409-
Requirement requirement, bool allowMissing) const {
409+
Requirement requirement,
410+
bool allowMissing,
411+
bool brokenPackBehavior) const {
410412
if (requirement.getFirstType()->hasTypeParameter()) {
411413
auto *genericEnv = getGenericEnvironment();
412414

415+
if (brokenPackBehavior) {
416+
// Swift 5.9 shipped with a bug here where this method would return
417+
// incorrect results. Maintain the old behavior specifically for two
418+
// call sites in the ASTMangler.
419+
if ((requirement.getKind() == RequirementKind::SameType ||
420+
requirement.getKind() == RequirementKind::Superclass) &&
421+
!requirement.getSecondType()->isTypeParameter() &&
422+
requirement.getSecondType().findIf([&](Type t) -> bool {
423+
return t->is<PackExpansionType>();
424+
})) {
425+
return false;
426+
}
427+
}
428+
413429
requirement = requirement.subst(
414430
QueryInterfaceTypeSubstitutions{genericEnv},
415431
LookUpConformanceInModule(),
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// RUN: %target-swift-frontend -emit-ir %s -target %target-swift-5.9-abi-triple | %FileCheck %s
2+
3+
public protocol P {
4+
associatedtype A
5+
}
6+
7+
public protocol Q {}
8+
9+
public class C<each T> {}
10+
11+
public struct GG1<A: P, each B: P> where A.A == C<repeat (each B).A> {}
12+
13+
extension GG1: Q where A: Q, repeat each B: Q {}
14+
15+
// This mangling is incorrect; the correct mangling is "$s29conditional_pack_requirements3GG1Vyxq_q_Qp_QPGAA1QA2aERzAaER_rlMc"
16+
// However, we retain the incorrect behavior for ABI compatibility.
17+
//
18+
// CHECK-LABEL: @"$s29conditional_pack_requirements3GG1Vyxq_q_Qp_QPGAA1QA2aERzAaER_AA1CCy1AAA1PPQy_q_Qp_QPGAhJRtzrlMc" =
19+
20+
21+
public struct GG2<each A: P> {
22+
public struct Nested<each B: P> where repeat (each A).A == (each B).A {}
23+
}
24+
25+
extension GG2.Nested: Q where repeat each A: Q, repeat each B: Q {}
26+
27+
// This mangling is correct.
28+
// CHECK-LABEL: @"$s29conditional_pack_requirements3GG2V6NestedVyxxQp_QP_qd__qd__Qp_QPGAA1QA2aGRzAaGRd__rlMc" =
29+
30+
31+
public struct GG3<A: P, each B: P> where A.A : C<repeat (each B).A> {}
32+
33+
extension GG3: Q where A: Q, repeat each B: Q {}
34+
35+
// This mangling is incorrect; the correct mangling is "$s29conditional_pack_requirements3GG3Vyxq_q_Qp_QPGAA1QA2aERzAaER_rlMc"
36+
// However, we retain the incorrect behavior for ABI compatibility.
37+
//
38+
// CHECK-LABEL: @"$s29conditional_pack_requirements3GG3Vyxq_q_Qp_QPGAA1QA2aERzAaER_AA1CCy1AAA1PPQy_q_Qp_QPGAhJRczrlMc" =
39+
40+
41+
public struct GG4<each A: P> {
42+
public struct Nested<each B: P> where repeat (each A).A : C<(each B).A> {}
43+
}
44+
45+
extension GG4.Nested: Q where repeat each A: Q, repeat each B: Q {}
46+
47+
// This mangling is correct.
48+
// CHECK-LABEL: @"$s29conditional_pack_requirements3GG4V6NestedVyxxQp_QP_qd__qd__Qp_QPGAA1QA2aGRzAaGRd__rlMc" =
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-emit-module-interface(%t/conditional_pack_requirements.swiftinterface) %s -target %target-swift-5.9-abi-triple
3+
// RUN: %FileCheck %s < %t/conditional_pack_requirements.swiftinterface
4+
5+
public protocol P {
6+
associatedtype A
7+
}
8+
9+
public protocol Q {}
10+
11+
public class C<each T> {}
12+
13+
public struct GG1<A: P, each B: P> where A.A == C<repeat (each B).A> {}
14+
15+
extension GG1: Q where A: Q, repeat each B: Q {}
16+
17+
// 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> {
18+
// CHECK-LABEL: extension conditional_pack_requirements.GG1 : conditional_pack_requirements.Q where A : conditional_pack_requirements.Q, repeat each B : conditional_pack_requirements.Q {
19+
20+
21+
public struct GG2<each A: P> {
22+
public struct Nested<each B: P> where repeat (each A).A == (each B).A {}
23+
}
24+
25+
extension GG2.Nested: Q where repeat each A: Q, repeat each B: Q {}
26+
27+
// CHECK-LABEL: public struct GG2<each A> where repeat each A : conditional_pack_requirements.P {
28+
// CHECK-LABEL: public struct Nested<each B> where repeat each B : conditional_pack_requirements.P, repeat (each A).A == (each B).A {
29+
// 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 {
30+
31+
32+
public struct GG3<A: P, each B: P> where A.A : C<repeat (each B).A> {}
33+
34+
extension GG3: Q where A: Q, repeat each B: Q {}
35+
36+
// 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> {
37+
// CHECK-LABEL: extension conditional_pack_requirements.GG3 : conditional_pack_requirements.Q where A : conditional_pack_requirements.Q, repeat each B : conditional_pack_requirements.Q {
38+
39+
40+
public struct GG4<each A: P> {
41+
public struct Nested<each B: P> where repeat (each A).A : C<(each B).A> {}
42+
}
43+
44+
extension GG4.Nested: Q where repeat each A: Q, repeat each B: Q {}
45+
46+
// CHECK-LABEL: public struct GG4<each A> where repeat each A : conditional_pack_requirements.P {
47+
// 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> {
48+
// 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 {

0 commit comments

Comments
 (0)