Skip to content

Commit 94e4e38

Browse files
committed
[GSB] Warn about redundant ': Any' type constraints
Such a stated constraint is always redundant. Resolves SR-8009.
1 parent 4ae85ac commit 94e4e38

File tree

6 files changed

+226
-25
lines changed

6 files changed

+226
-25
lines changed

include/swift/AST/GenericSignatureBuilder.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,9 +1276,12 @@ class GenericSignatureBuilder::RequirementSource final
12761276
ProtocolDecl *proto,
12771277
bool &derivedViaConcrete) const;
12781278

1279-
/// Retrieve a source location that corresponds to the requirement.
1279+
/// Retrieve a source location that corresponds to the requirement, if any.
12801280
SourceLoc getLoc() const;
12811281

1282+
/// Retrieve the source range for the requirement, if any.
1283+
SourceRange getSourceRange() const;
1284+
12821285
/// Compare two requirement sources to determine which has the more
12831286
/// optimal path.
12841287
///
@@ -1449,9 +1452,12 @@ class GenericSignatureBuilder::FloatingRequirementSource {
14491452
const RequirementSource *getSource(GenericSignatureBuilder &builder,
14501453
Type type) const;
14511454

1452-
/// Retrieve the source location for this requirement.
1455+
/// Retrieve the source location for this requirement, if any.
14531456
SourceLoc getLoc() const;
14541457

1458+
/// Retrieve the source range for this requirement, if any.
1459+
SourceRange getSourceRange() const;
1460+
14551461
/// Whether this is an explicitly-stated requirement.
14561462
bool isExplicit() const;
14571463

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 85 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,16 @@ SourceLoc RequirementSource::getLoc() const {
14691469
return SourceLoc();
14701470
}
14711471

1472+
SourceRange RequirementSource::getSourceRange() const {
1473+
if (auto requirementRepr = getRequirementRepr())
1474+
return requirementRepr->getSourceRange();
1475+
1476+
if (auto *typeRepr = getTypeRepr())
1477+
return typeRepr->getSourceRange();
1478+
1479+
return SourceRange();
1480+
}
1481+
14721482
/// Compute the path length of a requirement source, counting only the number
14731483
/// of \c ProtocolRequirement elements.
14741484
static unsigned sourcePathLength(const RequirementSource *source) {
@@ -1697,6 +1707,25 @@ SourceLoc FloatingRequirementSource::getLoc() const {
16971707
return SourceLoc();
16981708
}
16991709

1710+
SourceRange FloatingRequirementSource::getSourceRange() const {
1711+
// If this is a source for a protocol requirement, get the source range
1712+
// for that requirement.
1713+
if (kind == Kind::AbstractProtocol)
1714+
if (auto *written = protocolReq.written.dyn_cast<const RequirementRepr *>())
1715+
return written->getSourceRange();
1716+
1717+
if (auto *requirement = storage.dyn_cast<const RequirementRepr *>())
1718+
return requirement->getSourceRange();
1719+
1720+
if (auto *source = storage.dyn_cast<const RequirementSource *>())
1721+
return source->getSourceRange();
1722+
1723+
if (auto *typeRepr = storage.dyn_cast<const TypeRepr *>())
1724+
return typeRepr->getSourceRange();
1725+
1726+
return SourceRange();
1727+
}
1728+
17001729
bool FloatingRequirementSource::isExplicit() const {
17011730
switch (kind) {
17021731
case Explicit:
@@ -4146,20 +4175,29 @@ static ConstraintResult visitInherited(
41464175
llvm::function_ref<ConstraintResult(Type, const TypeRepr *)> visitType) {
41474176
// Local function that (recursively) adds inherited types.
41484177
ConstraintResult result = ConstraintResult::Resolved;
4149-
std::function<void(Type, const TypeRepr *)> visitInherited;
4150-
4151-
visitInherited = [&](Type inheritedType, const TypeRepr *typeRepr) {
4152-
// Decompose explicitly-written protocol compositions.
4153-
if (auto composition = dyn_cast_or_null<CompositionTypeRepr>(typeRepr)) {
4154-
if (auto compositionType
4155-
= inheritedType->getAs<ProtocolCompositionType>()) {
4156-
unsigned index = 0;
4157-
for (auto memberType : compositionType->getMembers()) {
4158-
visitInherited(memberType, composition->getTypes()[index]);
4159-
index++;
4160-
}
4178+
std::function<void(Type, const TypeRepr *, bool)> visitInherited;
41614179

4162-
return;
4180+
auto typeDecl = decl.dyn_cast<TypeDecl *>();
4181+
auto extDecl = decl.dyn_cast<ExtensionDecl *>();
4182+
auto &ctx = typeDecl ? typeDecl->getASTContext() : extDecl->getASTContext();
4183+
4184+
visitInherited = [&](Type inheritedType, const TypeRepr *typeRepr,
4185+
bool skipDecomposition) {
4186+
if (!skipDecomposition) {
4187+
// Decompose explicitly-written protocol compositions.
4188+
if (auto composition = dyn_cast_or_null<CompositionTypeRepr>(typeRepr)) {
4189+
if (auto compositionType =
4190+
inheritedType->getAs<ProtocolCompositionType>()) {
4191+
4192+
unsigned index = 0;
4193+
for (auto memberType : compositionType->getMembers()) {
4194+
visitInherited(memberType, composition->getTypes()[index],
4195+
skipDecomposition);
4196+
index++;
4197+
}
4198+
4199+
return;
4200+
}
41634201
}
41644202
}
41654203

@@ -4169,17 +4207,27 @@ static ConstraintResult visitInherited(
41694207
};
41704208

41714209
// Visit all of the inherited types.
4172-
auto typeDecl = decl.dyn_cast<TypeDecl *>();
4173-
auto extDecl = decl.dyn_cast<ExtensionDecl *>();
41744210
ArrayRef<TypeLoc> inheritedTypes = typeDecl ? typeDecl->getInherited()
41754211
: extDecl->getInherited();
41764212
for (unsigned index : indices(inheritedTypes)) {
41774213
Type inheritedType = typeDecl ? typeDecl->getInheritedType(index)
41784214
: extDecl->getInheritedType(index);
41794215
if (!inheritedType) continue;
41804216

4217+
// If the inherited type is 'Any', allow it to pass through verbatim, as
4218+
// GSB diagnoses redundant 'Any' constraints. The reason we do this check
4219+
// here instead of inside visitInherited is in order to avoid diagnosing on
4220+
// constraints such as ': Any & P'. This is because:
4221+
// - 1: It preserves parity with the behaviour for protocol composition
4222+
// constraints that appear in 'where' clauses (which aren't decomposed until
4223+
// after the redundant 'Any' diagnostic).
4224+
// - 2: The diagnosis of 'Any & ...' protocol composititions is better done
4225+
// as a separate broader diagnostic that warns on the usage of such
4226+
// compositions anywhere, not just in constraints.
4227+
bool skipDecomposition = inheritedType->isEqual(ctx.TheAnyType);
4228+
41814229
const auto &inherited = inheritedTypes[index];
4182-
visitInherited(inheritedType, inherited.getTypeRepr());
4230+
visitInherited(inheritedType, inherited.getTypeRepr(), skipDecomposition);
41834231
}
41844232

41854233
return result;
@@ -4711,14 +4759,19 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(
47114759
assert(constraintType && "No type to express resolved constraint?");
47124760
}
47134761

4762+
auto getSubjectInterfaceTy = [&] {
4763+
if (auto subjectType = subject.dyn_cast<Type>())
4764+
return subjectType;
4765+
4766+
return subject.get<PotentialArchetype *>()->getDependentType(
4767+
getGenericParams());
4768+
};
4769+
47144770
// Check whether we have a reasonable constraint type at all.
47154771
if (!constraintType->isExistentialType() &&
47164772
!constraintType->getClassOrBoundGenericClass()) {
47174773
if (source.getLoc().isValid() && !constraintType->hasError()) {
4718-
auto subjectType = subject.dyn_cast<Type>();
4719-
if (!subjectType)
4720-
subjectType = subject.get<PotentialArchetype *>()
4721-
->getDependentType(getGenericParams());
4774+
auto subjectType = getSubjectInterfaceTy();
47224775

47234776
Impl->HadAnyError = true;
47244777
Diags.diagnose(source.getLoc(), diag::requires_conformance_nonprotocol,
@@ -4780,6 +4833,18 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(
47804833
return ConstraintResult::Resolved;
47814834
}
47824835

4836+
// An explicit 'Any' constraint is redundant.
4837+
if (source.isExplicit() && constraintType->isEqual(Context.TheAnyType)) {
4838+
auto subjectType = getSubjectInterfaceTy();
4839+
SourceLoc diagLoc = source.getLoc();
4840+
4841+
Diags.diagnose(diagLoc, diag::redundant_conformance_constraint,
4842+
subjectType, constraintType)
4843+
.highlight(source.getSourceRange());
4844+
Diags.diagnose(diagLoc, diag::all_types_implicitly_conform_to,
4845+
constraintType);
4846+
}
4847+
47834848
// Protocol requirements.
47844849
if (constraintType->isExistentialType()) {
47854850
bool anyErrors = false;

test/Generics/function_decls.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
//===----------------------------------------------------------------------===//
66

77
func f0<T>(x: Int, y: Int, t: T) { }
8-
func f1<T : Any>(x: Int, y: Int, t: T) { }
8+
func f1<T : Any>(x: Int, y: Int, t: T) { } // expected-warning {{redundant conformance constraint 'T': 'Any'}}
9+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
10+
911
func f2<T : IteratorProtocol>(x: Int, y: Int, t: T) { }
1012
func f3<T : () -> ()>(x: Int, y: Int, t: T) { } // expected-error{{expected a class type or protocol-constrained type restricting 'T'}}
1113
func f4<T>(x: T, y: T) { }
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
protocol P1 {}
4+
protocol P2 {}
5+
6+
struct S01<T : Any> {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
7+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
8+
9+
struct S02<T> where T : Any {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
10+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
11+
12+
struct S04<T : P1> where T : Any {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
13+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
14+
15+
struct S05<T : Any> where T : P1 {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
16+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
17+
18+
struct S06<T : Any> where T : Any {}
19+
// expected-warning@-1 {{redundant conformance constraint 'T': 'Any'}}
20+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
21+
// expected-warning@-3 {{redundant conformance constraint 'T': 'Any'}}
22+
// expected-note@-4 {{all types implicitly conform to 'Any'}}
23+
24+
struct S07<T> where T : Any, T : P1 {}
25+
// expected-warning@-1 {{redundant conformance constraint 'T': 'Any'}}
26+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
27+
28+
struct S08<T> where T : P1, T : Any {}
29+
// expected-warning@-1 {{redundant conformance constraint 'T': 'Any'}}
30+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
31+
32+
struct S09<T> where T : P1, T : Any, T : P2 {}
33+
// expected-warning@-1 {{redundant conformance constraint 'T': 'Any'}}
34+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
35+
36+
struct S10<T> where T : P1, T : P2, T : Any {}
37+
// expected-warning@-1 {{redundant conformance constraint 'T': 'Any'}}
38+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
39+
40+
struct S11<T, U> {}
41+
extension S11 where T : Any {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
42+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
43+
44+
extension S11 where T : Any, T : P1 {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
45+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
46+
47+
extension S11 where T : Any, U : Any {}
48+
// expected-warning@-1 {{redundant conformance constraint 'T': 'Any'}}
49+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
50+
// expected-warning@-3 {{redundant conformance constraint 'U': 'Any'}}
51+
// expected-note@-4 {{all types implicitly conform to 'Any'}}
52+
53+
54+
protocol P3 {
55+
// expected-warning@-1 {{redundant conformance constraint 'Self.X1': 'Any'}}
56+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
57+
// expected-warning@-3 {{redundant conformance constraint 'Self.X2': 'Any'}}
58+
// expected-note@-4 {{all types implicitly conform to 'Any'}}
59+
60+
associatedtype X1 : Any
61+
associatedtype X2 where X2 : Any
62+
}
63+
64+
protocol P4 {
65+
// expected-warning@-1 {{redundant conformance constraint 'Self.X1': 'Any'}}
66+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
67+
// expected-warning@-3 {{redundant conformance constraint 'Self.X1': 'Any'}}
68+
// expected-note@-4 {{all types implicitly conform to 'Any'}}
69+
// expected-warning@-5 {{redundant conformance constraint 'Self.X2': 'Any'}}
70+
// expected-note@-6 {{all types implicitly conform to 'Any'}}
71+
// expected-warning@-7 {{redundant conformance constraint 'Self.X2': 'Any'}}
72+
// expected-note@-8 {{all types implicitly conform to 'Any'}}
73+
// expected-warning@-9 {{redundant conformance constraint 'Self.X3': 'Any'}}
74+
// expected-note@-10 {{all types implicitly conform to 'Any'}}
75+
// expected-warning@-11 {{redundant conformance constraint 'Self.X3': 'Any'}}
76+
// expected-note@-12 {{all types implicitly conform to 'Any'}}
77+
78+
associatedtype X1 : Any, Any
79+
associatedtype X2 where X2 : Any, X2 : Any
80+
associatedtype X3 where X3 : Any, X3 : Any, X3 : P1
81+
}
82+
83+
protocol P5 : Any {} // expected-warning {{redundant conformance constraint 'Self': 'Any'}}
84+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
85+
86+
protocol P6 : Any & Any {} // expected-warning {{redundant conformance constraint 'Self': 'Any'}}
87+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
88+
89+
protocol P7 : Any, Any {}
90+
// expected-warning@-1 {{redundant conformance constraint 'Self': 'Any'}}
91+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
92+
// expected-warning@-3 {{redundant conformance constraint 'Self': 'Any'}}
93+
// expected-note@-4 {{all types implicitly conform to 'Any'}}
94+
95+
func f1<T : Any>(_ x: T) {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
96+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
97+
98+
func f2<T>(_ x: T) where T : Any {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
99+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
100+
101+
typealias AnyAlias = Any
102+
func f3<T>(_ x: T) where T : AnyAlias {} // expected-warning {{redundant conformance constraint 'T': 'AnyAlias' (aka 'Any')}}
103+
// expected-note@-1 {{all types implicitly conform to 'AnyAlias' (aka 'Any')}}
104+
105+
func f4<T : Any & Any>(_ x: T) {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
106+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
107+
108+
func f5<T>(_ x: T) where T : Any & Any {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
109+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
110+
111+
typealias AnyAnyAlias = Any & Any
112+
func f6<T : AnyAnyAlias>(_ x: T) where T : Any {}
113+
// expected-warning@-1 {{redundant conformance constraint 'T': 'AnyAnyAlias' (aka 'Any')}}
114+
// expected-note@-2 {{all types implicitly conform to 'AnyAnyAlias' (aka 'Any')}}
115+
// expected-warning@-3 {{redundant conformance constraint 'T': 'Any'}}
116+
// expected-note@-4 {{all types implicitly conform to 'Any'}}
117+
118+
// Don't specifically warn on 'Any & P' constraints.
119+
// FIX-ME(SR-8082): Implement a general diagnostic for the usage of Any & P1.
120+
func f7<T : Any & P1>(_ x: T) {}
121+
func f8<T>(_ x: T) where T : P1 & Any {}
122+

test/Sema/diag_ambiguous_overloads.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ Variadic.foo(1.0, 2.0, 3.0) // expected-error {{cannot convert value of type 'Do
5858

5959
//=-------------- SR-7918 --------------=/
6060
class sr7918_Suit {
61-
static func foo<T: Any>(_ :inout T) {}
61+
static func foo<T: Any>(_ :inout T) {} // expected-warning {{redundant conformance constraint 'T': 'Any'}}
62+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
6263
static func foo() {}
6364
}
6465

test/type/protocol_composition.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@ typealias Any1 = protocol<> // expected-error {{'protocol<>' syntax has been rem
2626
typealias Any2 = protocol< > // expected-error {{'protocol<>' syntax has been removed; use 'Any' instead}}
2727

2828
// Okay to inherit a typealias for Any type.
29-
protocol P5 : Any { }
29+
protocol P5 : Any { } // expected-warning {{redundant conformance constraint 'Self': 'Any'}}
30+
// expected-note@-1 {{all types implicitly conform to 'Any'}}
31+
3032
protocol P6 : protocol<> { } // expected-error {{'protocol<>' syntax has been removed; use 'Any' instead}}
33+
// expected-warning@-1 {{redundant conformance constraint 'Self': 'Any'}}
34+
// expected-note@-2 {{all types implicitly conform to 'Any'}}
35+
3136
typealias P7 = Any & Any1
3237

3338
extension Int : P5 { }

0 commit comments

Comments
 (0)