Skip to content

Commit f6359e9

Browse files
committed
GSB: Replace 'self derived' check with a more sound notion of well-foundedness
Consider this example: protocol P1 { associatedtype A : P2 } protocol P2 { associatedtype A } func foo<T : P2>(_: T) where T.A : P1, T.A.A == T {} We cannot drop 'T : P2', even though it can be derived from T.A.A == T and Self.A : P2 in protocol P1, because we actually need the conformance T : P2 in order to recover the concrete type for T.A. This was handled via a hack which would ensure that the conformance path (T.A : P1)(Self.A : P2) was not a candidate derivation for T : P2, because T : P2 is one of the conformances used to construct the subject type T.A in the conformance path. This hack did not generalize to "cycles" of length greater than 1 however: func foo<T : P2, U : P2>(_: T, _: U) where T.A : P1, T.A.A == U, U.A : P1, U.A.A == T {} We used to drop both T : P2 and U : P2, because each one had a conformance path (U.A : P1)(Self.A : P2) and (T.A : P1)(Self.A : P2), respectively; however, once we drop T : P2 and U : P2, these two paths become invalid for the same reason that the concrete type for T.A and U.A can no longer be recovered. The correct fix is to recursively visit the subject type of each base requirement when evaluating a conformance path, and not consider any requirement whose subject type's parent type cannot be recovered. This proceeds recursively. In the worst case, this algorithm is exponential in the number of conformance requirements, but it is not always exponential, and a generic signature is not going to have a large number of conformance requirements anyway (hopefully). Also, the old logic in getMinimalConformanceSource() wasn't cheap either. Perhaps some clever minimization can speed this up too, but I'm going to focus on correctness first, before looking at performance here. Fixes rdar://problem/74890907.
1 parent 2d77260 commit f6359e9

File tree

2 files changed

+57
-9
lines changed

2 files changed

+57
-9
lines changed

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5931,6 +5931,42 @@ GenericSignatureBuilder::isValidRequirementDerivationPath(
59315931
// invalid once redundant requirements are dropped.
59325932
if (isRedundantExplicitRequirement(otherReq))
59335933
return None;
5934+
5935+
SWIFT_DEFER {
5936+
visited.erase(otherReq);
5937+
};
5938+
visited.insert(otherReq);
5939+
5940+
auto otherSubjectType = otherReq.getSubjectType();
5941+
if (auto *depMemType = otherSubjectType->getAs<DependentMemberType>()) {
5942+
// If 'req' is based on some other conformance requirement
5943+
// `T.[P.]A : Q', we want to make sure that we have a
5944+
// non-redundant derivation for 'T : P'.
5945+
auto baseType = depMemType->getBase();
5946+
auto *proto = depMemType->getAssocType()->getProtocol();
5947+
5948+
auto *equivClass = resolveEquivalenceClass(baseType,
5949+
ArchetypeResolutionKind::AlreadyKnown);
5950+
assert(equivClass &&
5951+
"Explicit requirement names an unknown equivalence class?");
5952+
5953+
auto found = equivClass->conformsTo.find(proto);
5954+
assert(found != equivClass->conformsTo.end());
5955+
5956+
bool foundValidDerivation = false;
5957+
for (const auto &constraint : found->second) {
5958+
if (isValidRequirementDerivationPath(
5959+
visited, RequirementKind::Conformance,
5960+
constraint.source, proto,
5961+
requirementSignatureSelfProto)) {
5962+
foundValidDerivation = true;
5963+
break;
5964+
}
5965+
}
5966+
5967+
if (!foundValidDerivation)
5968+
return None;
5969+
}
59345970
}
59355971

59365972
return result.front();
@@ -6024,15 +6060,6 @@ void GenericSignatureBuilder::computeRedundantRequirements(
60246060
req, found->second,
60256061
requirementSignatureSelfProto,
60266062
[&](const Constraint<ProtocolDecl *> &constraint) {
6027-
// FIXME: Remove this.
6028-
auto minimalSource =
6029-
constraint.source->getMinimalConformanceSource(
6030-
*this,
6031-
constraint.getSubjectDependentType({ }),
6032-
proto);
6033-
if (minimalSource != constraint.source)
6034-
return true;
6035-
60366063
return false;
60376064
});
60386065

test/Generics/rdar74890907.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %target-typecheck-verify-swift
2+
// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
3+
4+
protocol P1 {
5+
associatedtype A : P2
6+
}
7+
8+
protocol P2 {
9+
associatedtype A
10+
}
11+
12+
// CHECK: Generic signature: <T, U where T : P2, T == U.A.A, U == T.A.A, T.A : P1, U.A : P1>
13+
func testTU<T : P2, U : P2>(_: T, _: U) where T.A : P1, T.A.A == U, U.A : P1, U.A.A == T {}
14+
// expected-warning@-1 {{redundant conformance constraint 'U' : 'P2'}}
15+
// expected-note@-2 {{conformance constraint 'U' : 'P2' implied here}}
16+
17+
// CHECK: Generic signature: <T, U where T == U.A.A, U : P2, U == T.A.A, T.A : P1, U.A : P1>
18+
func testU<T, U : P2>(_: T, _: U) where T.A : P1, T.A.A == U, U.A : P1, U.A.A == T {}
19+
20+
// CHECK: Generic signature: <T, U where T : P2, T == U.A.A, U == T.A.A, T.A : P1, U.A : P1>
21+
func testT<T : P2, U>(_: T, _: U) where T.A : P1, T.A.A == U, U.A : P1, U.A.A == T {}

0 commit comments

Comments
 (0)