Skip to content

Commit 2c4cfdc

Browse files
slavapestovslavapestov
committed
RequirementMachine: Generating conformances prefers to delete non-explicit rules
This is a heuristic to ensure that conformance requirements remain in their original protocol if possible, when the protocol is part of a connected component consisting of multiple protocols.
1 parent dcefd70 commit 2c4cfdc

File tree

4 files changed

+194
-9
lines changed

4 files changed

+194
-9
lines changed

‎lib/AST/RequirementMachine/GeneratingConformances.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,13 @@ static const ProtocolDecl *getParentConformanceForTerm(Term lhs) {
607607
/// conformance rules.
608608
void RewriteSystem::computeGeneratingConformances(
609609
llvm::DenseSet<unsigned> &redundantConformances) {
610-
// All conformance rules, sorted by left hand side.
611-
SmallVector<std::pair<unsigned, Term>, 4> conformanceRules;
610+
// All conformance rules, sorted by (isExplicit(), getLHS()), with non-explicit
611+
// rules with longer left hand sides coming first.
612+
//
613+
// The idea here is that we want less canonical rules to be eliminated first,
614+
// but we prefer to eliminate non-explicit rules, in an attempt to keep protocol
615+
// conformance rules in the same protocol as they were originally defined in.
616+
SmallVector<unsigned, 4> conformanceRules;
612617

613618
// Maps a conformance rule to a conformance path deriving the subject type's
614619
// base type. For example, consider the following conformance rule:
@@ -640,8 +645,7 @@ void RewriteSystem::computeGeneratingConformances(
640645
if (!rule.isProtocolConformanceRule())
641646
continue;
642647

643-
auto lhs = rule.getLHS();
644-
conformanceRules.emplace_back(ruleID, lhs);
648+
conformanceRules.push_back(ruleID);
645649

646650
// Initially, every non-redundant conformance rule can be expressed
647651
// as itself.
@@ -655,6 +659,8 @@ void RewriteSystem::computeGeneratingConformances(
655659
continue;
656660
}
657661

662+
auto lhs = rule.getLHS();
663+
658664
// Record a parent path if the subject type itself requires a non-trivial
659665
// conformance path to derive.
660666
if (auto *parentProto = getParentConformanceForTerm(lhs)) {
@@ -696,14 +702,18 @@ void RewriteSystem::computeGeneratingConformances(
696702
// Sort the list of conformance rules in reverse order; we're going to try
697703
// to minimize away less canonical rules first.
698704
std::sort(conformanceRules.begin(), conformanceRules.end(),
699-
[&](const std::pair<unsigned, Term> &lhs,
700-
const std::pair<unsigned, Term> &rhs) -> bool {
701-
return lhs.second.compare(rhs.second, Context) > 0;
705+
[&](unsigned lhs, unsigned rhs) -> bool {
706+
const auto &lhsRule = getRule(lhs);
707+
const auto &rhsRule = getRule(rhs);
708+
709+
if (lhsRule.isExplicit() != rhsRule.isExplicit())
710+
return !lhsRule.isExplicit();
711+
712+
return lhsRule.getLHS().compare(rhsRule.getLHS(), Context) > 0;
702713
});
703714

704715
// Find a minimal set of generating conformances.
705-
for (const auto &pair : conformanceRules) {
706-
unsigned ruleID = pair.first;
716+
for (unsigned ruleID : conformanceRules) {
707717
const auto &paths = conformancePaths[ruleID];
708718

709719
bool isProtocolRefinement = protocolRefinements.count(ruleID) > 0;

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ RequirementMachine::computeMinimalRequirements() {
200200
assert(Protos.size() > 0);
201201
System.minimizeRewriteSystem();
202202

203+
if (Dump) {
204+
llvm::dbgs() << "Minimized rewrite system:\n";
205+
dump(llvm::dbgs());
206+
}
207+
203208
auto rules = System.getMinimizedRules(Protos);
204209

205210
// Note that we build 'result' by iterating over 'Protos' rather than

‎test/Generics/conformance_requirement_in_original_protocol.swift

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// RUN: %target-swift-frontend -typecheck %s -debug-generic-signatures -requirement-machine-protocol-signatures=verify 2>&1 | %FileCheck %s
2+
3+
public protocol P1 {
4+
associatedtype X
5+
}
6+
7+
public protocol P2 {}
8+
9+
////
10+
11+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q1a@
12+
// CHECK-LABEL: Requirement signature: <Self where Self == Self.Y.Z, Self.Y : Q1b>
13+
14+
public protocol Q1a {
15+
associatedtype Y : Q1b where Self == Self.Y.Z
16+
}
17+
18+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q1b@
19+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : P1, Self.Z : Q1a, Self.Z.X : P2>
20+
21+
public protocol Q1b {
22+
associatedtype Z : Q1a, P1 where Self.Z.X : P2
23+
}
24+
25+
////
26+
27+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q2a@
28+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : P1, Self.Z : Q2b, Self.Z.X : P2>
29+
30+
public protocol Q2a {
31+
associatedtype Z : Q2b, P1 where Self.Z.X : P2
32+
}
33+
34+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q2b@
35+
// CHECK-LABEL: Requirement signature: <Self where Self == Self.Y.Z, Self.Y : Q2a>
36+
37+
public protocol Q2b {
38+
associatedtype Y : Q2a where Self == Self.Y.Z
39+
}
40+
41+
////
42+
43+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q3a@
44+
// CHECK-LABEL: Requirement signature: <Self where Self : P1, Self == Self.Y.Z, Self.X : P2, Self.Y : Q3b>
45+
46+
public protocol Q3a : P1 {
47+
associatedtype Y : Q3b where Self == Self.Y.Z, Self.X : P2
48+
}
49+
50+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q3b@
51+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : Q3a>
52+
53+
public protocol Q3b {
54+
associatedtype Z : Q3a
55+
}
56+
57+
////
58+
59+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q4a@
60+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : Q4b>
61+
62+
public protocol Q4a {
63+
associatedtype Z : Q4b
64+
}
65+
66+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q4b@
67+
// CHECK-LABEL: Requirement signature: <Self where Self : P1, Self == Self.Y.Z, Self.X : P2, Self.Y : Q4a>
68+
69+
public protocol Q4b : P1 {
70+
associatedtype Y : Q4a where Self == Self.Y.Z, Self.X : P2
71+
}
72+
73+
////
74+
75+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q5a@
76+
// CHECK-LABEL: Requirement signature: <Self where Self == Self.Y.Z, Self.X : P2, Self.Y : Q5b>
77+
78+
public protocol Q5a {
79+
associatedtype Y : Q5b where Self == Self.Y.Z, Self.X : P2
80+
}
81+
82+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q5b@
83+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : P1, Self.Z : Q5a>
84+
85+
public protocol Q5b {
86+
associatedtype Z : Q5a, P1
87+
}
88+
89+
///
90+
91+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q6a@
92+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : P1, Self.Z : Q6b>
93+
94+
public protocol Q6a {
95+
associatedtype Z : Q6b, P1
96+
}
97+
98+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q6b@
99+
// CHECK-LABEL: Requirement signature: <Self where Self == Self.Y.Z, Self.X : P2, Self.Y : Q6a>
100+
101+
public protocol Q6b {
102+
associatedtype Y : Q6a where Self == Self.Y.Z, Self.X : P2
103+
}
104+
105+
////
106+
107+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q7a@
108+
// CHECK-LABEL: Requirement signature: <Self where Self : P1, Self == Self.Y.Z, Self.Y : Q7b>
109+
110+
public protocol Q7a : P1 {
111+
associatedtype Y : Q7b where Self == Self.Y.Z
112+
}
113+
114+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q7b@
115+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : Q7a, Self.Z.X : P2>
116+
117+
public protocol Q7b {
118+
associatedtype Z : Q7a where Self.Z.X : P2
119+
}
120+
121+
////
122+
123+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q8a@
124+
// CHECK-LABEL: Requirement signature: <Self where Self.Z : Q8b, Self.Z.X : P2>
125+
126+
public protocol Q8a {
127+
associatedtype Z : Q8b where Self.Z.X : P2
128+
}
129+
130+
// CHECK-LABEL: conformance_requirement_in_original_protocol.(file).Q8b@
131+
// CHECK-LABEL: Requirement signature: <Self where Self : P1, Self == Self.Y.Z, Self.Y : Q8a>
132+
133+
public protocol Q8b : P1 {
134+
associatedtype Y : Q8a where Self == Self.Y.Z
135+
}

‎test/Generics/sr9595.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %target-swift-frontend -debug-generic-signatures -requirement-machine-protocol-signatures=on -typecheck %s 2>&1 | %FileCheck %s
2+
3+
protocol R {}
4+
5+
protocol Q {
6+
associatedtype Assoc
7+
}
8+
9+
// CHECK-LABEL: sr9595.(file).P1@
10+
// CHECK-LABEL: Requirement signature: <Self where Self.T == Self.U.U, Self.U : P1, Self.U : Q, Self.U == Self.T.U, Self.U.Assoc : R>
11+
protocol P1 {
12+
associatedtype T where T == U.U
13+
associatedtype U : P1, Q where U.Assoc : R, U == T.U
14+
}
15+
16+
// CHECK-LABEL: sr9595.(file).P2@
17+
// CHECK-LABEL: Requirement signature: <Self where Self.T == Self.U.U, Self.U : P2, Self.U : Q, Self.U == Self.T.U, Self.U.Assoc : R>
18+
protocol P2 {
19+
associatedtype T : P2 where T == U.U
20+
associatedtype U : P2, Q where U.Assoc : R, U == T.U
21+
}
22+
23+
// CHECK-LABEL: sr9595.(file).P3@
24+
// CHECK-LABEL: Requirement signature: <Self where Self.T : Q, Self.T == Self.U.U, Self.U : P3, Self.U == Self.T.U, Self.T.Assoc : R>
25+
protocol P3 {
26+
associatedtype T : Q where T.Assoc : R, T == U.U
27+
associatedtype U : P3 where U == T.U
28+
}
29+
30+
// CHECK-LABEL: sr9595.(file).P4@
31+
// CHECK-LABEL: Requirement signature: <Self where Self.T : Q, Self.T == Self.U.U, Self.U : P4, Self.U == Self.T.U, Self.T.Assoc : R>
32+
protocol P4 {
33+
associatedtype T : P4, Q where T.Assoc : R, T == U.U
34+
associatedtype U : P4 where U.Assoc : R, U == T.U
35+
}

0 commit comments

Comments
 (0)