Skip to content

[Archetype builder] Suppress redundant constraints more effectively. #7228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 44 additions & 21 deletions lib/AST/ArchetypeBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,8 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete(

// Make sure the concrete type fulfills the superclass requirement
// of the archetype.
RequirementSource redundantSource(RequirementSource::Redundant,
Source.getLoc());
if (T->Superclass) {
if (!T->Superclass->isExactSuperclassOf(Concrete, getLazyResolver())) {
Diags.diagnose(Source.getLoc(), diag::type_does_not_inherit,
Expand All @@ -1422,6 +1424,10 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete(
.highlight(T->SuperclassSource->getLoc());
return true;
}

// The superclass requirement is made redundant by the concrete type
// assignment.
updateRequirementSource(*T->SuperclassSource, redundantSource);
}

// Recursively resolve the associated types to their concrete types.
Expand All @@ -1432,7 +1438,7 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete(
if (auto *concreteArchetype = Concrete->getAs<ArchetypeType>()) {
Type witnessType = concreteArchetype->getNestedType(nested.first);
addSameTypeRequirementToConcrete(nested.second.front(), witnessType,
Source);
redundantSource);
} else if (assocType) {
assert(conformances.count(assocType->getProtocol()) > 0
&& "missing conformance?");
Expand All @@ -1449,11 +1455,11 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete(
if (auto witnessPA = resolveArchetype(witnessType)) {
addSameTypeRequirementBetweenArchetypes(nested.second.front(),
witnessPA,
Source);
redundantSource);
} else {
addSameTypeRequirementToConcrete(nested.second.front(),
witnessType,
Source);
redundantSource);
}
}
}
Expand Down Expand Up @@ -2173,17 +2179,39 @@ void ArchetypeBuilder::enumerateRequirements(llvm::function_ref<
RequirementSource source)> f) {
// Create anchors for all of the potential archetypes.
// FIXME: This is because we might be missing some from the equivalence
// classes.
// classes. It is an egregious hack.
visitPotentialArchetypes([&](PotentialArchetype *archetype) {
archetype->getArchetypeAnchor(*this);
(void)archetype->getArchetypeAnchor(*this);
});

// Collect all archetypes, and sort them.
// Collect all archetypes.
SmallVector<PotentialArchetype *, 8> archetypes;
visitPotentialArchetypes([&](PotentialArchetype *archetype) {
archetypes.push_back(archetype);
});

// Remove any invalid potential archetypes or archetypes whose parents are
// concrete; they have no requirements.
archetypes.erase(
std::remove_if(archetypes.begin(), archetypes.end(),
[&](PotentialArchetype *archetype) -> bool {
// Invalid archetypes are never representatives in well-formed or
// corrected signature, so we don't need to visit them.
if (archetype->isInvalid())
return true;

// If there is a concrete type above us, there are no requirements to
// emit.
if (archetype->getParent() &&
hasConcreteTypeInPath(archetype->getParent()))
return true;

// Keep it.
return false;
}),
archetypes.end());

// Sort the archetypes in canonical order.
llvm::array_pod_sort(archetypes.begin(), archetypes.end(),
compareDependentTypes);

Expand All @@ -2203,32 +2231,27 @@ void ArchetypeBuilder::enumerateRequirements(llvm::function_ref<
};

for (auto *archetype : archetypes) {
// Invalid archetypes are never representatives in well-formed or corrected
// signature, so we don't need to visit them.
if (archetype->isInvalid())
continue;

// If this type is equivalent to a concrete type, emit the same-type
// constraint.
auto rep = archetype->getRepresentative();
if (auto concreteType = rep->getConcreteType()) {
f(RequirementKind::SameType, archetype, concreteType,
rep->getConcreteTypeSource());
continue;
}

// Check whether this archetype is one of the anchors within its
// connected component. If so, we may need to emit an archetype anchor.
// connected component. If so, we may need to emit a same-type constraint.
//
// FIXME: O(n) in the number of implied connected components within the
// equivalence class. The equivalence class should be small, but...
auto rep = archetype->getRepresentative();
auto componentAnchors = getSameTypeComponentAnchors(rep);
auto knownAnchor = std::find(componentAnchors.begin(),
componentAnchors.end(),
archetype);
std::function<void()> deferredSameTypeRequirement;

if (knownAnchor != componentAnchors.end()) {
// If this equivalance class is bound to a concrete type, equate the
// anchor with a concrete type.
if (auto concreteType = rep->getConcreteType()) {
f(RequirementKind::SameType, archetype, concreteType,
rep->getConcreteTypeSource());
continue;
}

// If we're at the last anchor in the component, do nothing;
auto nextAnchor = knownAnchor;
++nextAnchor;
Expand Down
34 changes: 34 additions & 0 deletions test/Generics/requirement_inference.swift
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,37 @@ extension P7 where AssocP6.Element : P6,
AssocP6.Element == AssocP7.AssocP6.Element {
func nestedSameType1() { }
}

protocol P8 {
associatedtype A
associatedtype B
}

protocol P9 : P8 {
associatedtype A
associatedtype B
}

protocol P10 {
associatedtype A
associatedtype C
}

// CHECK-LABEL: sameTypeConcrete1@
// CHECK: Canonical generic signature: <τ_0_0 where τ_0_0 : P10, τ_0_0 : P9, τ_0_0.A == X3, τ_0_0.B == Int, τ_0_0.C == Int>
func sameTypeConcrete1<T : P9 & P10>(_: T) where T.A == X3, T.C == T.B, T.C == Int { }

// CHECK-LABEL: sameTypeConcrete2@
// CHECK: Canonical generic signature: <τ_0_0 where τ_0_0 : P10, τ_0_0 : P9, τ_0_0.B == X3, τ_0_0.C == X3>
func sameTypeConcrete2<T : P9 & P10>(_: T) where T.B : X3, T.C == T.B, T.C == X3 { }

// Note: a standard-library-based stress test to make sure we don't inject
// any additional requirements.
// CHECK-LABEL: RangeReplaceableCollection.f()@
// CHECK: <τ_0_0 where τ_0_0 : MutableCollection, τ_0_0 : RangeReplaceableCollection, τ_0_0.SubSequence == MutableRangeReplaceableSlice<τ_0_0>>
extension RangeReplaceableCollection where
Self: MutableCollection,
Self.SubSequence == MutableRangeReplaceableSlice<Self>
{
func f() { }
}