Skip to content

Commit 5ec3857

Browse files
committed
[GSB] Remove self-derived same-type-to-concrete constraints.
Introduce an operation on RequirementSource to determine whether a constraint with such a source, when it lands on a given potential archetype, is "self-derived": e.g., the final constraint is derived from the original constraint. Remove such constraints from the system during finalization, because otherwise they would make the original constraint redundant. Fixes rdar://problem/30478915, although we still need to apply this same logic to other kinds of constraints in the system.
1 parent c2da971 commit 5ec3857

File tree

3 files changed

+76
-2
lines changed

3 files changed

+76
-2
lines changed

include/swift/AST/GenericSignatureBuilder.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,13 @@ class GenericSignatureBuilder::RequirementSource final
698698
/// to a protocol.
699699
bool isDerivedViaConcreteConformance() const;
700700

701+
/// Determine whether the given derived requirement \c source, when rooted at
702+
/// the potential archetype \c pa, is actually derived from the same
703+
/// requirement. Such "self-derived" requirements do not make the original
704+
/// requirement redundant, because without said original requirement, the
705+
/// derived requirement ceases to hold.
706+
bool isSelfDerivedSource(PotentialArchetype *pa) const;
707+
701708
/// Retrieve a source location that corresponds to the requirement.
702709
SourceLoc getLoc() const;
703710

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,55 @@ bool RequirementSource::isDerivedViaConcreteConformance() const {
234234
return false;
235235
}
236236

237+
bool RequirementSource::isSelfDerivedSource(PotentialArchetype *pa) const {
238+
// If it's not a derived requirement, it's not self-derived.
239+
if (!isDerivedRequirement()) return false;
240+
241+
// Collect the path of associated types from the root pa.
242+
SmallVector<AssociatedTypeDecl *, 4> assocTypes;
243+
PotentialArchetype *currentPA = nullptr;
244+
for (auto source = this; source; source = source->parent) {
245+
switch (source->kind) {
246+
case RequirementSource::Parent:
247+
assocTypes.push_back(source->getAssociatedType());
248+
break;
249+
250+
case RequirementSource::NestedTypeNameMatch:
251+
return false;
252+
253+
case RequirementSource::Explicit:
254+
case RequirementSource::Inferred:
255+
case RequirementSource::RequirementSignatureSelf:
256+
currentPA = source->getRootPotentialArchetype();
257+
break;
258+
259+
case RequirementSource::Concrete:
260+
case RequirementSource::ProtocolRequirement:
261+
case RequirementSource::Superclass:
262+
break;
263+
}
264+
}
265+
266+
assert(currentPA && "Missing root potential archetype");
267+
268+
// Check whether anything of the potential archetypes in the path are
269+
// equivalent to the end of the path.
270+
auto rep = pa->getRepresentative();
271+
for (auto assocType : reversed(assocTypes)) {
272+
// Check whether this potential archetype is in the same equivalence class.
273+
if (currentPA->getRepresentative() == rep) return true;
274+
275+
// Get the next nested type, but only if we've seen it before.
276+
// FIXME: Feels hacky.
277+
auto knownNested = currentPA->NestedTypes.find(assocType->getName());
278+
if (knownNested == currentPA->NestedTypes.end()) return false;
279+
currentPA = knownNested->second.front();
280+
}
281+
282+
// FIXME: currentPA == pa?
283+
return false;
284+
}
285+
237286
#define REQUIREMENT_SOURCE_FACTORY_BODY( \
238287
SourceKind, Parent, Storage, ExtraStorage, \
239288
NumPotentialArchetypes) \
@@ -2836,10 +2885,18 @@ void GenericSignatureBuilder::checkRedundantConcreteTypeConstraints(
28362885
Type concreteType = representative->getConcreteType();
28372886
assert(concreteType && "No concrete type to check");
28382887

2839-
// Sort the concrete constraints, so we get a deterministic ordering of
2840-
// diagnostics.
2888+
// Remove any self-derived constraints.
28412889
auto equivClass = representative->getOrCreateEquivalenceClass();
28422890
auto &concreteConstraints = equivClass->concreteTypeConstraints;
2891+
concreteConstraints.erase(
2892+
std::remove_if(concreteConstraints.begin(), concreteConstraints.end(),
2893+
[&](const ConcreteConstraint &constraint) {
2894+
return constraint.source->isSelfDerivedSource(constraint.archetype);
2895+
}),
2896+
concreteConstraints.end());
2897+
2898+
// Sort the concrete constraints, so we get a deterministic ordering of
2899+
// diagnostics.
28432900
llvm::array_pod_sort(concreteConstraints.begin(), concreteConstraints.end());
28442901

28452902
// Find a representative constraint to use for diagnostics.
@@ -2893,6 +2950,9 @@ void GenericSignatureBuilder::checkRedundantConcreteTypeConstraints(
28932950
representativeConstraint->archetype->
28942951
getDependentType(genericParams, /*allowUnresolved=*/true),
28952952
representativeConstraint->concreteType);
2953+
2954+
(void)representativeConstraint->source->isSelfDerivedSource(
2955+
representativeConstraint->archetype);
28962956
};
28972957

28982958
// Go through the concrete constraints looking for redundancies.

test/Generics/requirement_inference.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ extension RangeReplaceableCollection where
189189
func f() { }
190190
}
191191

192+
// CHECK-LABEL: X14.recursiveConcreteSameType
193+
// CHECK: Generic signature: <T, V where T == CountableRange<Int>>
194+
// CHECK-NEXT: Canonical generic signature: <τ_0_0, τ_1_0 where τ_0_0 == CountableRange<Int>>
195+
struct X14<T: Collection> where T.Iterator == IndexingIterator<T> {
196+
func recursiveConcreteSameType<V>(_: V) where T == CountableRange<Int> { }
197+
}
198+
192199
// rdar://problem/30478915
193200
protocol P11 {
194201
associatedtype A

0 commit comments

Comments
 (0)