Skip to content

Commit bd266b0

Browse files
committed
[GSB] Introduce type-rewriting rules only when we merge equivalence classes.
Instead of representing every same-type constraint we see as a rewrite rule, only record rewrite rules when we merge equivalence classes, and record rules that map the between the anchors of the equivalence classes. This gives us fewer, smaller rewrite rules that (by construction) build correct anchors.
1 parent c7d0e16 commit bd266b0

File tree

4 files changed

+56
-43
lines changed

4 files changed

+56
-43
lines changed

include/swift/AST/GenericSignatureBuilder.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,10 @@ class GenericSignatureBuilder {
445445
/// Note that we have added the nested type nestedPA
446446
void addedNestedType(PotentialArchetype *nestedPA);
447447

448-
/// Add a rewrite rule for a same-type constraint between the given
449-
/// types.
450-
void addSameTypeRewriteRule(PotentialArchetype *type1,
451-
PotentialArchetype *type2);
448+
/// Add a rewrite rule that makes \c otherPA a part of the given equivalence
449+
/// class.
450+
void addSameTypeRewriteRule(EquivalenceClass *equivClass,
451+
PotentialArchetype *otherPA);
452452

453453
/// \brief Add a new conformance requirement specifying that the given
454454
/// potential archetypes are equivalent.

lib/AST/GenericSignature.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,7 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath(
11201120
if (source->kind == RequirementSource::Superclass ||
11211121
source->kind == RequirementSource::Concrete) {
11221122
auto conformance = source->getProtocolConformance();
1123+
(void)conformance;
11231124
assert(conformance.getRequirement() == conformingProto);
11241125
path.path.push_back({source->getAffectedType(), conformingProto});
11251126
return;

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,19 +2149,42 @@ Type EquivalenceClass::getAnchor(
21492149
}
21502150

21512151
// Form the anchor.
2152+
bool updatedAnchor = false;
21522153
for (auto member : members) {
21532154
auto anchorType =
21542155
builder.getCanonicalTypeParameter(member->getDependentType(genericParams));
21552156
if (!anchorType) continue;
21562157

2158+
#ifndef NDEBUG
2159+
// Check that we get consistent results from all of the anchors.
2160+
if (updatedAnchor) {
2161+
assert(anchorType->isEqual(archetypeAnchorCache.anchor) &&
2162+
"Inconsistent anchor computation");
2163+
continue;
2164+
}
2165+
#endif
2166+
21572167
// Record the cache miss and update the cache.
21582168
++NumArchetypeAnchorCacheMisses;
21592169
archetypeAnchorCache.anchor = anchorType;
21602170
archetypeAnchorCache.lastGeneration = builder.Impl->Generation;
2161-
return substAnchor();
2171+
updatedAnchor = true;
2172+
2173+
#if NDEBUG
2174+
break;
2175+
#endif
21622176
}
21632177

2164-
llvm_unreachable("Unable to compute anchor");
2178+
// FIXME: Once we are no longer constructing potential archetypes with
2179+
// concrete nested types, we can turn this into assert(updatedAnchor);
2180+
if (!updatedAnchor) {
2181+
++NumArchetypeAnchorCacheMisses;
2182+
archetypeAnchorCache.anchor =
2183+
builder.getCanonicalTypeParameter(members.front()->getDependentType({ }));
2184+
archetypeAnchorCache.lastGeneration = builder.Impl->Generation;
2185+
}
2186+
2187+
return substAnchor();
21652188
}
21662189

21672190
Type EquivalenceClass::getTypeInContext(GenericSignatureBuilder &builder,
@@ -3401,39 +3424,29 @@ GenericSignatureBuilder::Implementation::getOrCreateRewriteTreeRoot(
34013424
return root;
34023425
}
34033426

3404-
void GenericSignatureBuilder::addSameTypeRewriteRule(PotentialArchetype *pa1,
3405-
PotentialArchetype *pa2){
3406-
auto pathOpt1 = RewritePath::createPath(pa1);
3407-
if (!pathOpt1) return;
3408-
3409-
auto pathOpt2 = RewritePath::createPath(pa2);
3410-
if (!pathOpt2) return;
3411-
3412-
auto path1 = std::move(pathOpt1).getValue();
3413-
auto path2 = std::move(pathOpt2).getValue();
3427+
void GenericSignatureBuilder::addSameTypeRewriteRule(
3428+
EquivalenceClass *equivClass,
3429+
PotentialArchetype *otherPA){
3430+
// Simplify both sides in the hope of uncovering a common path.
3431+
Type simplifiedType1 = equivClass->getAnchor(*this, { });
3432+
if (!simplifiedType1) return;
34143433

3415-
// Look for a common path.
3416-
auto prefix = path1.commonPath(path2);
3417-
3418-
// If we didn't find a common path, try harder.
3419-
Type simplifiedType1;
34203434
Type simplifiedType2;
3421-
if (!prefix) {
3422-
// Simplify both sides in the hope of uncovering a common path.
3423-
simplifiedType1 = getCanonicalTypeParameter(pa1->getDependentType({ }));
3424-
simplifiedType2 = getCanonicalTypeParameter(pa2->getDependentType({ }));
3425-
if (simplifiedType1->isEqual(simplifiedType2)) return;
3435+
if (auto otherEquivClass = otherPA->getEquivalenceClassIfPresent())
3436+
simplifiedType2 = otherEquivClass->getAnchor(*this, { });
3437+
else
3438+
simplifiedType2 = getCanonicalTypeParameter(otherPA->getDependentType({ }));
3439+
if (!simplifiedType2) return;
34263440

3427-
// Create new paths from the simplified types.
3428-
path1 = *RewritePath::createPath(simplifiedType1);
3429-
path2 = *RewritePath::createPath(simplifiedType2);
3441+
// We already effectively have this rewrite rule.
3442+
if (simplifiedType1->isEqual(simplifiedType2)) return;
34303443

3431-
// Find a common path.
3432-
prefix = path1.commonPath(path2);
3433-
}
3444+
auto path1 = *RewritePath::createPath(simplifiedType1);
3445+
auto path2 = *RewritePath::createPath(simplifiedType2);
34343446

3435-
// When we have a common prefix, form a rewrite rule using relative paths.
3436-
if (prefix) {
3447+
// Look for a common prefix. When we have one, form a rewrite rule using
3448+
// relative paths.
3449+
if (auto prefix = path1.commonPath(path2)) {
34373450
// Find the better relative rewrite path.
34383451
RelativeRewritePath relPath1
34393452
= path1.getPath().slice(prefix->getPath().size());
@@ -3469,11 +3482,11 @@ void GenericSignatureBuilder::addSameTypeRewriteRule(PotentialArchetype *pa1,
34693482
Type firstBase =
34703483
GenericTypeParamType::get(path1.getBase()->Depth, path1.getBase()->Index,
34713484
getASTContext());
3472-
auto equivClass =
3485+
auto baseEquivClass =
34733486
resolveEquivalenceClass(firstBase, ArchetypeResolutionKind::WellFormed);
3474-
assert(equivClass && "Base cannot be resolved?");
3487+
assert(baseEquivClass && "Base cannot be resolved?");
34753488

3476-
auto root = Impl->getOrCreateRewriteTreeRoot(equivClass);
3489+
auto root = Impl->getOrCreateRewriteTreeRoot(baseEquivClass);
34773490
root->addRewriteRule(path1.getPath(), path2);
34783491
}
34793492

@@ -4539,9 +4552,6 @@ GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes(
45394552
PotentialArchetype *OrigT2,
45404553
const RequirementSource *Source)
45414554
{
4542-
// Add a rewrite rule based on the given same-type constraint.
4543-
addSameTypeRewriteRule(OrigT1, OrigT2);
4544-
45454555
// Record the same-type constraint, and bail out if it was already known.
45464556
if (!OrigT1->getOrCreateEquivalenceClass(*this)
45474557
->recordSameTypeConstraint(OrigT1, OrigT2, Source))
@@ -4561,10 +4571,12 @@ GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes(
45614571
std::swap(OrigT1, OrigT2);
45624572
}
45634573

4564-
// Merge the equivalence classes.
4574+
// Add a rewrite rule to map T2 down to the anchor.
45654575
auto equivClass = T1->getOrCreateEquivalenceClass(*this);
4566-
equivClass->modified(*this);
4576+
addSameTypeRewriteRule(equivClass, T2);
45674577

4578+
// Merge the equivalence classes.
4579+
equivClass->modified(*this);
45684580
auto equivClass1Members = equivClass->members;
45694581
auto equivClass2Members = T2->getEquivalenceClassMembers();
45704582
for (auto equiv : equivClass2Members)

test/Generics/sr5726.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ protocol Foo
77
var bar: Bar { get }
88
}
99

10-
extension Foo where Self: Collection, Bar: Collection, Self.SubSequence == Bar.SubSequence, Self.Index == Bar.Index
10+
extension Foo where Self: Collection, Bar: Collection, Self.SubSequence == Bar.SubSequence, /*redundant: */Self.Index == Bar.Index
1111
{
1212
subscript(_ bounds: Range<Index>) -> SubSequence
1313
{

0 commit comments

Comments
 (0)