Skip to content

Commit 65f27d3

Browse files
committed
RequirementMachine: Fix subtle corner case where we could insert duplicate rewrite rules
RewriteSystem::addRule() did two things before adding the rewrite rule: - Simplify both sides. If both sides are now equal, discard the rule. - If the last symbol on the left hand side was a superclass or concrete type symbol, simplify the substitution terms in that symbol. The problem is that the second step can produce a term which can be further simplified, and in particular, one that is exactly equal to the left hand side of some other rule. To fix this, swap the order of the two steps. The only wrinkle is now we have to check for a concrete type symbol at the end of _both_ the left hand side and right hand side, since we don't orient the rule until we simplify both sides. I don't have a reduced test case for this one, but it was revealed by compiler_crashers_2_fixed/0109-sr4737.swift after I introduced the trie.
1 parent bc398ae commit 65f27d3

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

lib/AST/RequirementMachine/RewriteSystem.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,14 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs) {
6262
assert(!lhs.empty());
6363
assert(!rhs.empty());
6464

65-
// Simplify the rule as much as possible with the rules we have so far.
65+
// First, simplify terms appearing inside concrete substitutions before
66+
// doing anything else.
67+
if (lhs.back().isSuperclassOrConcreteType())
68+
lhs.back() = simplifySubstitutionsInSuperclassOrConcreteSymbol(lhs.back());
69+
else if (rhs.back().isSuperclassOrConcreteType())
70+
rhs.back() = simplifySubstitutionsInSuperclassOrConcreteSymbol(rhs.back());
71+
72+
// Now simplify both sides as much as possible with the rules we have so far.
6673
//
6774
// This avoids unnecessary work in the completion algorithm.
6875
simplify(lhs);
@@ -79,9 +86,6 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs) {
7986
if (result < 0)
8087
std::swap(lhs, rhs);
8188

82-
if (lhs.back().isSuperclassOrConcreteType())
83-
lhs.back() = simplifySubstitutionsInSuperclassOrConcreteSymbol(lhs.back());
84-
8589
assert(lhs.compare(rhs, Protos) > 0);
8690

8791
if (DebugAdd) {

0 commit comments

Comments
 (0)