Skip to content

Commit 3837b4d

Browse files
hborlahborla
committed
[RequirementMachine] Diagnose early redundant requirements.
1 parent 203734b commit 3837b4d

File tree

5 files changed

+110
-11
lines changed

5 files changed

+110
-11
lines changed

‎include/swift/AST/Requirement.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ struct RequirementError {
134134
InvalidConformance,
135135
ConcreteTypeMismatch,
136136
NonTypeParameter,
137+
RedundantRequirement,
137138
} kind;
138139

139140
union {
@@ -148,6 +149,8 @@ struct RequirementError {
148149
} concreteTypeMismatch;
149150

150151
Type nonTypeParameter;
152+
153+
Requirement redundantRequirement;
151154
};
152155

153156
SourceLoc loc;
@@ -181,6 +184,13 @@ struct RequirementError {
181184
error.nonTypeParameter = subject;
182185
return error;
183186
}
187+
188+
static RequirementError forRedundantRequirement(Requirement req,
189+
SourceLoc loc) {
190+
RequirementError error(Kind::RedundantRequirement, loc);
191+
error.redundantRequirement = req;
192+
return error;
193+
}
184194
};
185195

186196
} // end namespace swift

‎lib/AST/RequirementMachine/RequirementLowering.cpp

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,44 +63,63 @@ static void desugarSameTypeRequirement(Type lhs, Type rhs, SourceLoc loc,
6363
SmallVectorImpl<RequirementError> &errors;
6464

6565
public:
66+
bool recordedErrors = false;
67+
bool recordedRequirements = false;
68+
6669
explicit Matcher(SourceLoc loc,
6770
SmallVectorImpl<Requirement> &result,
6871
SmallVectorImpl<RequirementError> &errors)
6972
: loc(loc), result(result), errors(errors) {}
7073

74+
bool alwaysMismatchTypeParameters() const { return true; }
75+
7176
bool mismatch(TypeBase *firstType, TypeBase *secondType,
7277
Type sugaredFirstType) {
7378
if (firstType->isTypeParameter() && secondType->isTypeParameter()) {
7479
result.emplace_back(RequirementKind::SameType,
7580
firstType, secondType);
81+
recordedRequirements = true;
7682
return true;
7783
}
7884

7985
if (firstType->isTypeParameter()) {
8086
result.emplace_back(RequirementKind::SameType,
8187
firstType, secondType);
88+
recordedRequirements = true;
8289
return true;
8390
}
8491

8592
if (secondType->isTypeParameter()) {
8693
result.emplace_back(RequirementKind::SameType,
8794
secondType, firstType);
95+
recordedRequirements = true;
8896
return true;
8997
}
9098

9199
errors.push_back(
92100
RequirementError::forConcreteTypeMismatch(firstType,
93101
secondType,
94102
loc));
103+
recordedErrors = true;
95104
return true;
96105
}
97106
} matcher(loc, result, errors);
98107

99108
if (lhs->hasError() || rhs->hasError())
100109
return;
101110

102-
// FIXME: Record redundancy and diagnose upstream
103111
(void) matcher.match(lhs, rhs);
112+
113+
// If neither side is directly a type parameter, the type parameter
114+
// must be in structural position where the enclosing type is redundant.
115+
if (!lhs->isTypeParameter() && !rhs->isTypeParameter() &&
116+
!matcher.recordedErrors) {
117+
// FIXME: Add a tailored error message when requirements were
118+
// recorded, e.g. Array<Int> == Array<T>. The outer type is
119+
// redundant, but the inner requirement T == Int is not.
120+
errors.push_back(RequirementError::forRedundantRequirement(
121+
{RequirementKind::SameType, lhs, rhs}, loc));
122+
}
104123
}
105124

106125
static void desugarSuperclassRequirement(Type subjectType,
@@ -109,8 +128,14 @@ static void desugarSuperclassRequirement(Type subjectType,
109128
SmallVectorImpl<Requirement> &result,
110129
SmallVectorImpl<RequirementError> &errors) {
111130
if (!subjectType->isTypeParameter()) {
112-
errors.push_back(
113-
RequirementError::forNonTypeParameter(subjectType, loc));
131+
if (constraintType->isExactSuperclassOf(subjectType)) {
132+
errors.push_back(RequirementError::forRedundantRequirement(
133+
{RequirementKind::Superclass, subjectType, constraintType}, loc));
134+
} else {
135+
errors.push_back(
136+
RequirementError::forNonTypeParameter(subjectType, loc));
137+
}
138+
114139
return;
115140
}
116141

@@ -123,8 +148,14 @@ static void desugarLayoutRequirement(Type subjectType,
123148
SmallVectorImpl<Requirement> &result,
124149
SmallVectorImpl<RequirementError> &errors) {
125150
if (!subjectType->isTypeParameter()) {
126-
errors.push_back(
127-
RequirementError::forNonTypeParameter(subjectType, loc));
151+
if (subjectType->isAnyClassReferenceType()) {
152+
errors.push_back(RequirementError::forRedundantRequirement(
153+
{RequirementKind::Layout, subjectType, layout}, loc));
154+
} else {
155+
errors.push_back(
156+
RequirementError::forNonTypeParameter(subjectType, loc));
157+
}
158+
128159
return;
129160
}
130161

@@ -163,7 +194,9 @@ static void desugarConformanceRequirement(Type subjectType, Type constraintType,
163194
return;
164195
}
165196

166-
// FIXME: Diagnose a redundancy.
197+
errors.push_back(RequirementError::forRedundantRequirement(
198+
{RequirementKind::Conformance, subjectType, constraintType}, loc));
199+
167200
assert(conformance.isConcrete());
168201
auto *concrete = conformance.getConcrete();
169202

@@ -493,12 +526,13 @@ void swift::rewriting::realizeInheritedRequirements(
493526
}
494527
}
495528

496-
void swift::rewriting::diagnoseRequirementErrors(
529+
bool swift::rewriting::diagnoseRequirementErrors(
497530
ASTContext &ctx, SmallVectorImpl<RequirementError> &errors,
498531
bool allowConcreteGenericParams) {
532+
bool diagnosedError = false;
499533
if (ctx.LangOpts.RequirementMachineProtocolSignatures !=
500534
RequirementMachineMode::Enabled)
501-
return;
535+
return diagnosedError;
502536

503537
for (auto error : errors) {
504538
SourceLoc loc = error.loc;
@@ -520,6 +554,7 @@ void swift::rewriting::diagnoseRequirementErrors(
520554

521555
ctx.Diags.diagnose(loc, diag::requires_conformance_nonprotocol,
522556
subjectType, constraintString);
557+
diagnosedError = true;
523558

524559
auto getNameWithoutSelf = [&](std::string subjectTypeName) {
525560
std::string selfSubstring = "Self.";
@@ -549,6 +584,7 @@ void swift::rewriting::diagnoseRequirementErrors(
549584
if (!type1->hasError() && !type2->hasError()) {
550585
ctx.Diags.diagnose(loc, diag::requires_same_concrete_type,
551586
type1, type2);
587+
diagnosedError = true;
552588
}
553589

554590
break;
@@ -557,10 +593,41 @@ void swift::rewriting::diagnoseRequirementErrors(
557593
case RequirementError::Kind::NonTypeParameter: {
558594
ctx.Diags.diagnose(loc, diag::requires_not_suitable_archetype,
559595
error.nonTypeParameter);
596+
diagnosedError = true;
597+
break;
598+
}
599+
600+
case RequirementError::Kind::RedundantRequirement: {
601+
auto requirement = error.redundantRequirement;
602+
switch (requirement.getKind()) {
603+
case RequirementKind::SameType:
604+
ctx.Diags.diagnose(loc, diag::redundant_same_type_to_concrete,
605+
requirement.getFirstType(),
606+
requirement.getSecondType());
607+
break;
608+
case RequirementKind::Conformance:
609+
ctx.Diags.diagnose(loc, diag::redundant_conformance_constraint,
610+
requirement.getFirstType(),
611+
requirement.getProtocolDecl());
612+
break;
613+
case RequirementKind::Superclass:
614+
ctx.Diags.diagnose(loc, diag::redundant_superclass_constraint,
615+
requirement.getFirstType(),
616+
requirement.getSecondType());
617+
break;
618+
case RequirementKind::Layout:
619+
ctx.Diags.diagnose(loc, diag::redundant_layout_constraint,
620+
requirement.getFirstType(),
621+
requirement.getLayoutConstraint());
622+
break;
623+
}
624+
560625
break;
561626
}
562627
}
563628
}
629+
630+
return diagnosedError;
564631
}
565632

566633
ArrayRef<StructuralRequirement>

‎lib/AST/RequirementMachine/RequirementLowering.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ void realizeInheritedRequirements(TypeDecl *decl, Type type,
5757
SmallVectorImpl<StructuralRequirement> &result,
5858
SmallVectorImpl<RequirementError> &errors);
5959

60-
void diagnoseRequirementErrors(ASTContext &ctx,
60+
bool diagnoseRequirementErrors(ASTContext &ctx,
6161
SmallVectorImpl<RequirementError> &errors,
6262
bool allowConcreteGenericParams);
6363

‎lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -631,9 +631,9 @@ InferredGenericSignatureRequestRQM::evaluate(
631631
machine->computeMinimalGenericSignatureRequirements();
632632

633633
auto result = GenericSignature::get(genericParams, minimalRequirements);
634-
bool hadError = machine->hadError() || !errors.empty();
634+
bool hadError = machine->hadError();
635635

636-
diagnoseRequirementErrors(ctx, errors, allowConcreteGenericParams);
636+
hadError |= diagnoseRequirementErrors(ctx, errors, allowConcreteGenericParams);
637637

638638
// FIXME: Handle allowConcreteGenericParams
639639

‎test/Generics/requirement_machine_diagnostics.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,25 @@ func badTypeConformance4<T>(_: T) where @escaping (inout T) throws -> () : Equal
4040
func badTypeConformance5<T>(_: T) where T & Sequence : EqualComparable { }
4141
// expected-error@-1 2 {{non-protocol, non-class type 'T' cannot be used within a protocol-constrained type}}
4242
// expected-error@-2{{type 'Sequence' in conformance requirement does not refer to a generic parameter or associated type}}
43+
44+
func badTypeConformance6<T>(_: T) where [T] : Collection { }
45+
// expected-warning@-1{{redundant conformance constraint '[T]' : 'Collection'}}
46+
47+
func concreteSameTypeRedundancy<T>(_: T) where Int == Int {}
48+
// expected-warning@-1{{redundant same-type constraint 'Int' == 'Int'}}
49+
50+
func concreteSameTypeRedundancy<T>(_: T) where Array<Int> == Array<T> {}
51+
// expected-warning@-1{{redundant same-type constraint 'Array<Int>' == 'Array<T>'}}
52+
53+
protocol P {}
54+
struct S: P {}
55+
func concreteConformanceRedundancy<T>(_: T) where S : P {}
56+
// expected-warning@-1{{redundant conformance constraint 'S' : 'P'}}
57+
58+
class C {}
59+
func concreteLayoutRedundancy<T>(_: T) where C : AnyObject {}
60+
// expected-warning@-1{{redundant constraint 'C' : 'AnyObject'}}
61+
62+
class C2: C {}
63+
func concreteSubclassRedundancy<T>(_: T) where C2 : C {}
64+
// expected-warning@-1{{redundant superclass constraint 'C2' : 'C'}}

0 commit comments

Comments
 (0)