Skip to content

Commit 0be55c1

Browse files
slavapestovslavapestov
committed
GSB: Use redundant requirement graph in enumerateRequirements()
1 parent 72bd35a commit 0be55c1

File tree

6 files changed

+217
-60
lines changed

6 files changed

+217
-60
lines changed

‎include/swift/AST/GenericSignatureBuilder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ class GenericSignatureBuilder {
675675
Constraint<T> checkConstraintList(
676676
TypeArrayView<GenericTypeParamType> genericParams,
677677
std::vector<Constraint<T>> &constraints,
678+
RequirementKind kind,
678679
llvm::function_ref<bool(const Constraint<T> &)>
679680
isSuitableRepresentative,
680681
llvm::function_ref<
@@ -700,6 +701,7 @@ class GenericSignatureBuilder {
700701
Constraint<T> checkConstraintList(
701702
TypeArrayView<GenericTypeParamType> genericParams,
702703
std::vector<Constraint<T>> &constraints,
704+
RequirementKind kind,
703705
llvm::function_ref<bool(const Constraint<T> &)>
704706
isSuitableRepresentative,
705707
llvm::function_ref<

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 57 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5429,7 +5429,9 @@ namespace {
54295429
/// Retrieve the representative constraint that will be used for diagnostics.
54305430
template<typename T>
54315431
Optional<Constraint<T>> findRepresentativeConstraint(
5432+
GenericSignatureBuilder &builder,
54325433
ArrayRef<Constraint<T>> constraints,
5434+
RequirementKind kind,
54335435
llvm::function_ref<bool(const Constraint<T> &)>
54345436
isSuitableRepresentative) {
54355437
Optional<Constraint<T>> fallbackConstraint;
@@ -5453,6 +5455,26 @@ namespace {
54535455
continue;
54545456
}
54555457

5458+
if (kind != RequirementKind::SameType) {
5459+
// We prefer non-redundant explicit constraints over everything else.
5460+
bool thisIsNonRedundantExplicit =
5461+
(!constraint.source->isDerivedNonRootRequirement() &&
5462+
!builder.isRedundantExplicitRequirement(
5463+
ExplicitRequirement::fromExplicitConstraint(
5464+
kind, constraint)));
5465+
bool representativeIsNonRedundantExplicit =
5466+
(!representativeConstraint->source->isDerivedNonRootRequirement() &&
5467+
!builder.isRedundantExplicitRequirement(
5468+
ExplicitRequirement::fromExplicitConstraint(
5469+
kind, *representativeConstraint)));
5470+
5471+
if (thisIsNonRedundantExplicit != representativeIsNonRedundantExplicit) {
5472+
if (thisIsNonRedundantExplicit)
5473+
representativeConstraint = constraint;
5474+
continue;
5475+
}
5476+
}
5477+
54565478
// We prefer derived constraints to non-derived constraints.
54575479
bool thisIsDerived = constraint.source->isDerivedRequirement();
54585480
bool representativeIsDerived =
@@ -6302,7 +6324,9 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
63026324
// Try to find an exact constraint that matches 'other'.
63036325
auto repConstraint =
63046326
findRepresentativeConstraint<Type>(
6327+
*this,
63056328
equivClass->sameTypeConstraints,
6329+
RequirementKind::SameType,
63066330
[pa, other](const Constraint<Type> &constraint) {
63076331
return (constraint.isSubjectEqualTo(pa) &&
63086332
constraint.value->isEqual(other->getDependentType())) ||
@@ -6315,7 +6339,9 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
63156339
if (!repConstraint) {
63166340
repConstraint =
63176341
findRepresentativeConstraint<Type>(
6342+
*this,
63186343
equivClass->sameTypeConstraints,
6344+
RequirementKind::SameType,
63196345
[](const Constraint<Type> &constraint) {
63206346
return true;
63216347
});
@@ -6466,6 +6492,7 @@ template<typename T>
64666492
Constraint<T> GenericSignatureBuilder::checkConstraintList(
64676493
TypeArrayView<GenericTypeParamType> genericParams,
64686494
std::vector<Constraint<T>> &constraints,
6495+
RequirementKind kind,
64696496
llvm::function_ref<bool(const Constraint<T> &)>
64706497
isSuitableRepresentative,
64716498
llvm::function_ref<
@@ -6475,7 +6502,7 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
64756502
conflictingDiag,
64766503
Diag<Type, T> redundancyDiag,
64776504
Diag<unsigned, Type, T> otherNoteDiag) {
6478-
return checkConstraintList<T, T>(genericParams, constraints,
6505+
return checkConstraintList<T, T>(genericParams, constraints, kind,
64796506
isSuitableRepresentative, checkConstraint,
64806507
conflictingDiag, redundancyDiag,
64816508
otherNoteDiag,
@@ -6570,6 +6597,7 @@ template<typename T, typename DiagT>
65706597
Constraint<T> GenericSignatureBuilder::checkConstraintList(
65716598
TypeArrayView<GenericTypeParamType> genericParams,
65726599
std::vector<Constraint<T>> &constraints,
6600+
RequirementKind kind,
65736601
llvm::function_ref<bool(const Constraint<T> &)>
65746602
isSuitableRepresentative,
65756603
llvm::function_ref<
@@ -6591,7 +6619,8 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
65916619

65926620
// Find a representative constraint.
65936621
auto representativeConstraint =
6594-
findRepresentativeConstraint<T>(constraints, isSuitableRepresentative);
6622+
findRepresentativeConstraint<T>(*this, constraints, kind,
6623+
isSuitableRepresentative);
65956624

65966625
// Local function to provide a note describing the representative constraint.
65976626
auto noteRepresentativeConstraint = [&] {
@@ -6748,7 +6777,7 @@ void GenericSignatureBuilder::checkConformanceConstraints(
67486777
removeSelfDerived(*this, entry.second, entry.first);
67496778

67506779
checkConstraintList<ProtocolDecl *, ProtocolDecl *>(
6751-
genericParams, entry.second,
6780+
genericParams, entry.second, RequirementKind::Conformance,
67526781
[](const Constraint<ProtocolDecl *> &constraint) {
67536782
return true;
67546783
},
@@ -7376,7 +7405,7 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
73767405
if (constraints.empty()) continue;
73777406

73787407
checkConstraintList<Type, Type>(
7379-
genericParams, constraints,
7408+
genericParams, constraints, RequirementKind::SameType,
73807409
[](const Constraint<Type> &) { return true; },
73817410
[](const Constraint<Type> &constraint) {
73827411
// Ignore nested-type-name-match constraints.
@@ -7569,7 +7598,7 @@ void GenericSignatureBuilder::checkConcreteTypeConstraints(
75697598
resolveDependentMemberTypes(*this, equivClass->concreteType);
75707599

75717600
checkConstraintList<Type>(
7572-
genericParams, equivClass->concreteTypeConstraints,
7601+
genericParams, equivClass->concreteTypeConstraints, RequirementKind::SameType,
75737602
[&](const ConcreteConstraint &constraint) {
75747603
if (constraint.value->isEqual(resolvedConcreteType))
75757604
return true;
@@ -7612,7 +7641,7 @@ void GenericSignatureBuilder::checkSuperclassConstraints(
76127641

76137642
auto representativeConstraint =
76147643
checkConstraintList<Type>(
7615-
genericParams, equivClass->superclassConstraints,
7644+
genericParams, equivClass->superclassConstraints, RequirementKind::Superclass,
76167645
[&](const ConcreteConstraint &constraint) {
76177646
if (constraint.value->isEqual(resolvedSuperclass))
76187647
return true;
@@ -7692,7 +7721,7 @@ void GenericSignatureBuilder::checkLayoutConstraints(
76927721
if (!equivClass->layout) return;
76937722

76947723
checkConstraintList<LayoutConstraint>(
7695-
genericParams, equivClass->layoutConstraints,
7724+
genericParams, equivClass->layoutConstraints, RequirementKind::Layout,
76967725
[&](const Constraint<LayoutConstraint> &constraint) {
76977726
return constraint.value == equivClass->layout;
76987727
},
@@ -7720,20 +7749,22 @@ bool GenericSignatureBuilder::isRedundantExplicitRequirement(
77207749
}
77217750

77227751
namespace {
7723-
/// Retrieve the best requirement source from a set of constraints.
77247752
template<typename T>
7725-
const RequirementSource *
7726-
getBestConstraintSource(ArrayRef<Constraint<T>> constraints,
7727-
llvm::function_ref<bool(const T&)> matches) {
7728-
const RequirementSource *bestSource = nullptr;
7729-
for (const auto &constraint : constraints) {
7730-
if (!matches(constraint.value)) continue;
7753+
bool hasNonRedundantRequirementSource(ArrayRef<Constraint<T>> constraints,
7754+
RequirementKind kind,
7755+
GenericSignatureBuilder &builder) {
7756+
for (auto constraint : constraints) {
7757+
if (constraint.source->isDerivedRequirement())
7758+
continue;
77317759

7732-
if (!bestSource || constraint.source->compare(bestSource) < 0)
7733-
bestSource = constraint.source;
7760+
auto req = ExplicitRequirement::fromExplicitConstraint(kind, constraint);
7761+
if (builder.isRedundantExplicitRequirement(req))
7762+
continue;
7763+
7764+
return true;
77347765
}
77357766

7736-
return bestSource;
7767+
return false;
77377768
}
77387769

77397770
using SameTypeComponentRef = std::pair<EquivalenceClass *, unsigned>;
@@ -7855,27 +7886,19 @@ void GenericSignatureBuilder::enumerateRequirements(
78557886
if (equivClass->superclass &&
78567887
!equivClass->recursiveSuperclassType &&
78577888
!equivClass->superclass->hasError()) {
7858-
auto bestSource =
7859-
getBestConstraintSource<Type>(equivClass->superclassConstraints,
7860-
[&](const Type &type) {
7861-
return type->isEqual(equivClass->superclass);
7862-
});
7863-
7864-
if (!bestSource->isDerivedRequirement()) {
7889+
if (hasNonRedundantRequirementSource<Type>(
7890+
equivClass->superclassConstraints,
7891+
RequirementKind::Superclass, *this)) {
78657892
recordRequirement(RequirementKind::Superclass,
78667893
subjectType, equivClass->superclass);
78677894
}
78687895
}
78697896

78707897
// If we have a layout constraint, produce a layout requirement.
78717898
if (equivClass->layout) {
7872-
auto bestSource = getBestConstraintSource<LayoutConstraint>(
7873-
equivClass->layoutConstraints,
7874-
[&](const LayoutConstraint &layout) {
7875-
return layout == equivClass->layout;
7876-
});
7877-
7878-
if (!bestSource->isDerivedRequirement()) {
7899+
if (hasNonRedundantRequirementSource<LayoutConstraint>(
7900+
equivClass->layoutConstraints,
7901+
RequirementKind::Layout, *this)) {
78797902
recordRequirement(RequirementKind::Layout,
78807903
subjectType, equivClass->layout);
78817904
}
@@ -7885,14 +7908,10 @@ void GenericSignatureBuilder::enumerateRequirements(
78857908
SmallVector<ProtocolDecl *, 4> protocols;
78867909

78877910
for (const auto &conforms : equivClass->conformsTo) {
7888-
auto *bestSource = getBestConstraintSource<ProtocolDecl *>(
7889-
conforms.second,
7890-
[&](ProtocolDecl *proto) {
7891-
return proto == conforms.first;
7892-
});
7893-
7894-
if (!bestSource->isDerivedRequirement())
7911+
if (hasNonRedundantRequirementSource<ProtocolDecl *>(
7912+
conforms.second, RequirementKind::Conformance, *this)) {
78957913
protocols.push_back(conforms.first);
7914+
}
78967915
}
78977916

78987917
// Sort the protocols in canonical order.

‎test/Generics/rdar62903491.swift

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// RUN: %target-typecheck-verify-swift
2+
// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
3+
4+
protocol P {
5+
associatedtype X : P
6+
}
7+
8+
// Anything that mentions 'T : P' minimizes to 'U : P'.
9+
10+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P'}}
11+
// expected-note@+1 {{conformance constraint 'U': 'P' implied here}}
12+
func oneProtocol1<T, U>(_: T, _: U) where T : P, U : P, T.X == U, U.X == T {}
13+
// CHECK-LABEL: oneProtocol1
14+
// CHECK: Generic signature: <T, U where T : P, T == U.X, U == T.X>
15+
16+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P'}}
17+
// expected-note@+1 {{conformance constraint 'U': 'P' implied here}}
18+
func oneProtocol2<T, U>(_: T, _: U) where U : P, T : P, T.X == U, U.X == T {}
19+
// CHECK-LABEL: oneProtocol2
20+
// CHECK: Generic signature: <T, U where T : P, T == U.X, U == T.X>
21+
22+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P'}}
23+
// expected-note@+1 {{conformance constraint 'U': 'P' implied here}}
24+
func oneProtocol3<T, U>(_: T, _: U) where T : P, T.X == U, U : P, U.X == T {}
25+
// CHECK-LABEL: oneProtocol3
26+
// CHECK: Generic signature: <T, U where T : P, T == U.X, U == T.X>
27+
28+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P'}}
29+
// expected-note@+1 {{conformance constraint 'U': 'P' implied here}}
30+
func oneProtocol4<T, U>(_: T, _: U) where U : P, T.X == U, T : P, U.X == T {}
31+
// CHECK-LABEL: oneProtocol4
32+
// CHECK: Generic signature: <T, U where T : P, T == U.X, U == T.X>
33+
34+
func oneProtocol5<T, U>(_: T, _: U) where T : P, T.X == U, U.X == T {}
35+
// CHECK-LABEL: oneProtocol5
36+
// CHECK: Generic signature: <T, U where T : P, T == U.X, U == T.X>
37+
38+
func oneProtocol6<T, U>(_: T, _: U) where T.X == U, U.X == T, T : P {}
39+
// CHECK-LABEL: oneProtocol6
40+
// CHECK: Generic signature: <T, U where T : P, T == U.X, U == T.X>
41+
42+
// Anything that mentions 'U : P' but not 'T : P' minimizes to 'U : P'.
43+
44+
// FIXME: Need to emit warning here too
45+
func oneProtocol7<T, U>(_: T, _: U) where U : P, T.X == U, U.X == T {}
46+
// CHECK-LABEL: oneProtocol7
47+
// CHECK: Generic signature: <T, U where T == U.X, U : P, U == T.X>
48+
49+
// FIXME: Need to emit warning here too
50+
func oneProtocol8<T, U>(_: T, _: U) where T.X == U, U.X == T, U : P {}
51+
// CHECK-LABEL: oneProtocol8
52+
// CHECK: Generic signature: <T, U where T == U.X, U : P, U == T.X>
53+
54+
protocol P1 {
55+
associatedtype X : P2
56+
}
57+
58+
protocol P2 {
59+
associatedtype Y : P1
60+
}
61+
62+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P2'}}
63+
// expected-note@+1 {{conformance constraint 'U': 'P2' implied here}}
64+
func twoProtocols1<T, U>(_: T, _: U) where T : P1, U : P2, T.X == U, U.Y == T {}
65+
// CHECK-LABEL: twoProtocols1
66+
// CHECK: Generic signature: <T, U where T : P1, T == U.Y, U == T.X>
67+
68+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P2'}}
69+
// expected-note@+1 {{conformance constraint 'U': 'P2' implied here}}
70+
func twoProtocols2<T, U>(_: T, _: U) where U : P2, T : P1, T.X == U, U.Y == T {}
71+
// CHECK-LABEL: twoProtocols2
72+
// CHECK: Generic signature: <T, U where T : P1, T == U.Y, U == T.X>
73+
74+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P2'}}
75+
// expected-note@+1 {{conformance constraint 'U': 'P2' implied here}}
76+
func twoProtocols3<T, U>(_: T, _: U) where T : P1, T.X == U, U : P2, U.Y == T {}
77+
// CHECK-LABEL: twoProtocols3
78+
// CHECK: Generic signature: <T, U where T : P1, T == U.Y, U == T.X>
79+
80+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P2'}}
81+
// expected-note@+1 {{conformance constraint 'U': 'P2' implied here}}
82+
func twoProtocols4<T, U>(_: T, _: U) where U : P2, T.X == U, T : P1, U.Y == T {}
83+
// CHECK-LABEL: twoProtocols4
84+
// CHECK: Generic signature: <T, U where T : P1, T == U.Y, U == T.X>
85+
86+
// expected-warning@+2 {{redundant conformance constraint 'U': 'P2'}}
87+
// expected-note@+1 {{conformance constraint 'U': 'P2' implied here}}
88+
func twoProtocols5<T, U>(_: T, _: U) where T : P1, T.X == U, U.Y == T, U : P2 {}
89+
// CHECK-LABEL: twoProtocols5
90+
// CHECK: Generic signature: <T, U where T : P1, T == U.Y, U == T.X>
91+
92+
// expected-warning@+2 {{redundant conformance constraint 'T': 'P1'}}
93+
// expected-note@+1 {{conformance constraint 'T': 'P1' implied here}}
94+
func twoProtocols6<T, U>(_: T, _: U) where U : P2, T.X == U, U.Y == T, T : P1 {}
95+
// CHECK-LABEL: twoProtocols6
96+
// CHECK: Generic signature: <T, U where T == U.Y, U : P2, U == T.X>

‎test/Generics/same_type_constraints.swift

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -378,22 +378,6 @@ typealias NotAnInt = Double
378378
extension X11 where NotAnInt == Int { }
379379
// expected-error@-1{{generic signature requires types 'NotAnInt' (aka 'Double') and 'Int' to be the same}}
380380

381-
382-
struct X12<T> { }
383-
384-
protocol P12 {
385-
associatedtype A
386-
associatedtype B
387-
}
388-
389-
func testP12a<T: P12>(_: T) where T.A == X12<Int>, T.A == X12<T.B>, T.B == Int { }
390-
// expected-warning@-1{{redundant same-type constraint 'T.B' == 'Int'}}
391-
// expected-note@-2{{same-type constraint 'T.B' == 'Int' written here}}
392-
393-
func testP12b<T: P12>(_: T) where T.B == Int, T.A == X12<T.B>, X12<T.B> == T.A { }
394-
// expected-warning@-1{{redundant same-type constraint 'T.A' == 'X12<Int>'}}
395-
// expected-note@-2{{same-type constraint 'T.A' == 'X12<Int>' written here}}
396-
397381
// rdar://45307061 - dropping delayed same-type constraints when merging
398382
// equivalence classes
399383

0 commit comments

Comments
 (0)