Skip to content

Commit cd262be

Browse files
swift-ciswift-ci
authored
Merge pull request #8955 from DougGregor/associated-type-override-hints
2 parents 119ca8a + 225a26d commit cd262be

17 files changed

+237
-54
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,6 +1652,13 @@ NOTE(previous_same_type_constraint, none,
16521652
"%select{written here|implied here|inferred from type here}0",
16531653
(unsigned, Type, Type))
16541654

1655+
WARNING(inherited_associated_type_redecl,none,
1656+
"redeclaration of associated type %0 from protocol %1 is better "
1657+
"expressed as a 'where' clause on the protocol", (DeclName, Type))
1658+
WARNING(typealias_override_associated_type,none,
1659+
"typealias overriding associated type %0 from protocol %1 is better "
1660+
"expressed as same-type constraint on the protocol", (DeclName, Type))
1661+
16551662
ERROR(generic_param_access,none,
16561663
"%0 %select{must be declared %select{"
16571664
"%select{private|fileprivate|internal|%error|%error}3|private or fileprivate}4"

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 148 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2457,6 +2457,72 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
24572457
}
24582458
}
24592459

2460+
// Collect all of the inherited associated types and typealiases in the
2461+
// inherited protocols (recursively).
2462+
DenseMap<DeclName, TinyPtrVector<TypeDecl *>> inheritedTypeDecls;
2463+
{
2464+
Proto->walkInheritedProtocols(
2465+
[&](ProtocolDecl *inheritedProto) -> TypeWalker::Action {
2466+
if (inheritedProto == Proto) return TypeWalker::Action::Continue;
2467+
2468+
for (auto req : getProtocolMembers(inheritedProto)) {
2469+
if (auto typeReq = dyn_cast<TypeDecl>(req))
2470+
inheritedTypeDecls[typeReq->getFullName()].push_back(typeReq);
2471+
}
2472+
return TypeWalker::Action::Continue;
2473+
});
2474+
}
2475+
2476+
// Local function to find the insertion point for the protocol's "where"
2477+
// clause, as well as the string to start the insertion ("where" or ",");
2478+
auto getProtocolWhereLoc = [&]() -> std::pair<SourceLoc, const char *> {
2479+
// Already has a trailing where clause.
2480+
if (auto trailing = Proto->getTrailingWhereClause())
2481+
return { trailing->getRequirements().back().getSourceRange().End, ", " };
2482+
2483+
// Inheritance clause.
2484+
return { Proto->getInherited().back().getSourceRange().End, " where " };
2485+
};
2486+
2487+
// Retrieve the set of requirements that a given associated type declaration
2488+
// produces, in the form that would be seen in the where clause.
2489+
auto getAssociatedTypeReqs = [&](AssociatedTypeDecl *assocType,
2490+
const char *start) {
2491+
std::string result;
2492+
{
2493+
llvm::raw_string_ostream out(result);
2494+
out << start;
2495+
interleave(assocType->getInherited(), [&](TypeLoc inheritedType) {
2496+
out << assocType->getFullName() << ": ";
2497+
if (auto inheritedTypeRepr = inheritedType.getTypeRepr())
2498+
inheritedTypeRepr->print(out);
2499+
else
2500+
inheritedType.getType().print(out);
2501+
}, [&] {
2502+
out << ", ";
2503+
});
2504+
}
2505+
return result;
2506+
};
2507+
2508+
// Retrieve the requirement that a given typealias introduces when it
2509+
// overrides an inherited associated type with the same name, as a string
2510+
// suitable for use in a where clause.
2511+
auto getTypeAliasReq = [&](TypeAliasDecl *typealias, const char *start) {
2512+
std::string result;
2513+
{
2514+
llvm::raw_string_ostream out(result);
2515+
out << start;
2516+
out << typealias->getFullName() << " == ";
2517+
if (auto underlyingTypeRepr =
2518+
typealias->getUnderlyingTypeLoc().getTypeRepr())
2519+
underlyingTypeRepr->print(out);
2520+
else
2521+
typealias->getUnderlyingTypeLoc().getType().print(out);
2522+
}
2523+
return result;
2524+
};
2525+
24602526
// Add requirements for each of the associated types.
24612527
for (auto Member : getProtocolMembers(Proto)) {
24622528
if (auto AssocType = dyn_cast<AssociatedTypeDecl>(Member)) {
@@ -2475,14 +2541,88 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
24752541
addRequirement(&req, innerSource, &protocolSubMap, protoModule);
24762542
}
24772543
}
2478-
} else if (auto TypeAlias = dyn_cast<TypeAliasDecl>(Member)) {
2479-
// FIXME: this should check that the typealias is makes sense (e.g. has
2480-
// the same/compatible type as typealiases in parent protocols) and
2481-
// set-up any same type requirements required. Forcing the PA to be
2482-
// created with getNestedType is currently worse than useless due to the
2483-
// 'recursive decl validation' FIXME in that function: it creates an
2484-
// unresolved PA that prints an error later.
2485-
(void)TypeAlias;
2544+
2545+
// Check whether we inherited any types with the same name.
2546+
auto knownInherited = inheritedTypeDecls.find(AssocType->getFullName());
2547+
if (knownInherited == inheritedTypeDecls.end()) continue;
2548+
2549+
bool shouldWarnAboutRedeclaration =
2550+
Source->kind == RequirementSource::RequirementSignatureSelf &&
2551+
AssocType->getDefaultDefinitionLoc().isNull();
2552+
for (auto inheritedType : knownInherited->getSecond()) {
2553+
// If we have inherited associated type...
2554+
if (auto inheritedAssocTypeDecl =
2555+
dyn_cast<AssociatedTypeDecl>(inheritedType)) {
2556+
// FIXME: Wire up same-type constraint.
2557+
2558+
// Complain about the first redeclaration.
2559+
if (shouldWarnAboutRedeclaration) {
2560+
auto inheritedFromProto = inheritedAssocTypeDecl->getProtocol();
2561+
auto fixItWhere = getProtocolWhereLoc();
2562+
Diags.diagnose(AssocType,
2563+
diag::inherited_associated_type_redecl,
2564+
AssocType->getFullName(),
2565+
inheritedFromProto->getDeclaredInterfaceType())
2566+
.fixItInsertAfter(
2567+
fixItWhere.first,
2568+
getAssociatedTypeReqs(AssocType, fixItWhere.second))
2569+
.fixItRemove(AssocType->getSourceRange());
2570+
2571+
Diags.diagnose(inheritedAssocTypeDecl, diag::decl_declared_here,
2572+
inheritedAssocTypeDecl->getFullName());
2573+
2574+
shouldWarnAboutRedeclaration = false;
2575+
}
2576+
2577+
continue;
2578+
}
2579+
2580+
// FIXME: this is a weird situation.
2581+
}
2582+
2583+
inheritedTypeDecls.erase(knownInherited);
2584+
continue;
2585+
}
2586+
2587+
if (auto typealias = dyn_cast<TypeAliasDecl>(Member)) {
2588+
// Check whether we inherited any types with the same name.
2589+
auto knownInherited = inheritedTypeDecls.find(typealias->getFullName());
2590+
if (knownInherited == inheritedTypeDecls.end()) continue;
2591+
2592+
bool shouldWarnAboutRedeclaration =
2593+
Source->kind == RequirementSource::RequirementSignatureSelf;
2594+
2595+
for (auto inheritedType : knownInherited->getSecond()) {
2596+
// If we have inherited associated type...
2597+
if (auto inheritedAssocTypeDecl =
2598+
dyn_cast<AssociatedTypeDecl>(inheritedType)) {
2599+
// FIXME: Wire up same-type constraint.
2600+
2601+
// Warn that one should use where clauses for this.
2602+
if (shouldWarnAboutRedeclaration) {
2603+
auto inheritedFromProto = inheritedAssocTypeDecl->getProtocol();
2604+
auto fixItWhere = getProtocolWhereLoc();
2605+
Diags.diagnose(typealias,
2606+
diag::typealias_override_associated_type,
2607+
typealias->getFullName(),
2608+
inheritedFromProto->getDeclaredInterfaceType())
2609+
.fixItInsertAfter(fixItWhere.first,
2610+
getTypeAliasReq(typealias, fixItWhere.second))
2611+
.fixItRemove(typealias->getSourceRange());
2612+
Diags.diagnose(inheritedAssocTypeDecl, diag::decl_declared_here,
2613+
inheritedAssocTypeDecl->getFullName());
2614+
2615+
shouldWarnAboutRedeclaration = false;
2616+
}
2617+
2618+
continue;
2619+
}
2620+
2621+
// FIXME: More typealiases
2622+
}
2623+
2624+
inheritedTypeDecls.erase(knownInherited);
2625+
continue;
24862626
}
24872627
}
24882628

‎stdlib/public/core/Integers.swift.gyb

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,11 +2348,8 @@ ${operatorComment('&' + x.operator, True)}
23482348
//===----------------------------------------------------------------------===//
23492349

23502350
/// An integer type that can represent only nonnegative values.
2351-
public protocol UnsignedInteger : BinaryInteger {
2352-
/// A type that can represent the absolute value of any possible value of the
2353-
/// conforming type.
2354-
associatedtype Magnitude : BinaryInteger
2355-
}
2351+
public protocol UnsignedInteger : BinaryInteger
2352+
where Magnitude : BinaryInteger { }
23562353

23572354
extension UnsignedInteger {
23582355
/// The magnitude of this value.
@@ -2458,11 +2455,8 @@ extension UnsignedInteger where Self : FixedWidthInteger {
24582455
//===----------------------------------------------------------------------===//
24592456

24602457
/// An integer type that can represent both positive and negative values.
2461-
public protocol SignedInteger : BinaryInteger, SignedNumeric {
2462-
/// A type that can represent the absolute value of any possible value of the
2463-
/// conforming type.
2464-
associatedtype Magnitude : BinaryInteger
2465-
2458+
public protocol SignedInteger : BinaryInteger, SignedNumeric
2459+
where Magnitude : BinaryInteger {
24662460
// These requirements are for the source code compatibility with Swift 3
24672461
static func _maskingAdd(_ lhs: Self, _ rhs: Self) -> Self
24682462
static func _maskingSubtract(_ lhs: Self, _ rhs: Self) -> Self

‎test/Constraints/generic_overload.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %target-typecheck-verify-swift
22

3-
protocol P1 { associatedtype Assoc }
4-
protocol P2 : P1 { associatedtype Assoc }
3+
protocol P1 { associatedtype Assoc } // expected-note{{declared here}}
4+
protocol P2 : P1 { associatedtype Assoc } // expected-warning{{redeclaration of associated type}}
55
protocol P3 { }
66

77
struct X1 : P1 { typealias Assoc = X3 }

‎test/Generics/associated_type_typo.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ func typoFunc3<T : P1>(x: TypoType, y: (T.Assoc) -> ()) { // expected-error{{use
6060
typealias Element_<S: Sequence> = S.Iterator.Element
6161

6262
public protocol _Indexable1 {
63-
associatedtype Slice
63+
associatedtype Slice // expected-note{{declared here}}
6464
}
6565
public protocol Indexable : _Indexable1 {
66-
associatedtype Slice : _Indexable1
66+
associatedtype Slice : _Indexable1 // expected-warning{{redeclaration of associated type 'Slice'}}
6767
}
6868

6969
protocol Pattern {

‎test/Generics/associated_type_where_clause.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,16 @@ protocol P {
138138

139139
// Lookup of same-named associated types aren't ambiguous in this context.
140140
protocol P1 {
141-
associatedtype A
141+
associatedtype A // expected-note 2{{declared here}}
142142
}
143143

144144
protocol P2: P1 {
145-
associatedtype A
145+
associatedtype A // expected-warning{{redeclaration of associated type}}
146146
associatedtype B where A == B
147147
}
148148

149149
protocol P3: P1 {
150-
associatedtype A
150+
associatedtype A // expected-warning{{redeclaration of associated type}}
151151
}
152152

153153
protocol P4 {

‎test/Generics/associated_type_where_clause_hints.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %target-typecheck-verify-swift -typecheck %s -verify
2+
3+
protocol P0 { }
4+
protocol P0b { }
5+
6+
struct X0 : P0 { }
7+
8+
struct X1 { }
9+
10+
protocol P1 {
11+
associatedtype A // expected-note 2{{'A' declared here}}
12+
}
13+
14+
// A typealias in a subprotocol should be written as a same-type
15+
// requirement.
16+
protocol P2 : P1 {
17+
typealias A = X1 // expected-warning{{typealias overriding associated type 'A' from protocol 'P1' is better expressed as same-type constraint on the protocol}}{{17-17= where A == X1}}{{3-20=}}
18+
}
19+
20+
// A redeclaration of an associated type that adds type/layout requirements
21+
// should be written via a where clause.
22+
protocol P3a : P1 {
23+
associatedtype A: P0, P0b // expected-warning{{redeclaration of associated type 'A' from protocol 'P1' is better expressed as a 'where' clause on the protocol}}{{18-18= where A: P0, A: P0b}}{{3-29=}}
24+
}
25+
26+
// ... unless it has adds a default type witness
27+
protocol P3b : P1 {
28+
associatedtype A: P0 = X0 // note: no warning
29+
}
30+
31+
protocol P4: P1 {
32+
associatedtype B // expected-note{{'B' declared here}}
33+
}
34+
35+
protocol P5: P4 where A: P0 {
36+
typealias B = X1 // expected-warning{{typealias overriding associated type 'B' from protocol 'P4' is better expressed as same-type constraint on the protocol}}{{28-28=, B == X1}}{{3-20=}}
37+
}
38+
39+

‎test/Generics/canonicalization.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
protocol P0 { }
55

66
protocol P {
7-
associatedtype A
7+
associatedtype A // expected-note{{declared here}}
88
}
99

1010
protocol Q : P {
11-
associatedtype A
11+
associatedtype A // expected-warning{{redeclaration of associated type 'A' from protocol 'P' is better expressed as a 'where' clause on the protocol}}
1212
}
1313

1414
func f<T>(t: T) where T : P, T : Q, T.A : P0 { } // expected-note{{'f(t:)' previously declared here}}

‎test/Generics/conformance_access_path.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ func testPaths3<V: P5>(_ v: V) {
5858

5959
protocol P6Unordered: P5Unordered { // expected-note{{conformance constraint 'Self.A': 'P0' implied here}}
6060
associatedtype A: P0 // expected-warning{{redundant conformance constraint 'Self.A': 'P0'}}
61+
// expected-warning@-1{{redeclaration of associated type 'A'}}
6162
}
6263

6364
protocol P5Unordered {
64-
associatedtype A: P0
65+
associatedtype A: P0 // expected-note{{'A' declared here}}
6566

6667
func getA() -> A
6768
}

‎test/Generics/protocol_requirement_signatures.swift

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ protocol P3 {}
2222
// CHECK-NEXT: Requirement signature: <Self where Self.X : P1>
2323
// CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0.X : P1>
2424
protocol Q1 {
25-
associatedtype X: P1
25+
associatedtype X: P1 // expected-note 2{{declared here}}
2626
}
2727

2828
// inheritance
@@ -36,15 +36,16 @@ protocol Q2: Q1 {}
3636
// CHECK-NEXT: Requirement signature: <Self where Self : Q1>
3737
// CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0 : Q1>
3838
protocol Q3: Q1 {
39-
associatedtype X
39+
associatedtype X // expected-warning{{redeclaration of associated type 'X'}}
4040
}
4141

4242
// inheritance adding a new conformance
4343
// CHECK-LABEL: .Q4@
4444
// CHECK-NEXT: Requirement signature: <Self where Self : Q1, Self.X : P2>
4545
// CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0 : Q1, τ_0_0.X : P2>
4646
protocol Q4: Q1 {
47-
associatedtype X: P2
47+
associatedtype X: P2 // expected-warning{{redeclaration of associated type 'X'}}
48+
// expected-note@-1 2{{'X' declared here}}
4849
}
4950

5051
// multiple inheritance
@@ -60,12 +61,13 @@ protocol Q5: Q2, Q3, Q4 {}
6061
protocol Q6: Q2, // expected-note{{conformance constraint 'Self.X': 'P1' implied here}}
6162
Q3, Q4 {
6263
associatedtype X: P1 // expected-warning{{redundant conformance constraint 'Self.X': 'P1'}}
64+
// expected-warning@-1{{redeclaration of associated type 'X' from protocol 'Q4' is}}
6365
}
6466

6567
// multiple inheritance with a new conformance
6668
// CHECK-LABEL: .Q7@
6769
// CHECK-NEXT: Requirement signature: <Self where Self : Q2, Self : Q3, Self : Q4, Self.X : P3>
6870
// CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0 : Q2, τ_0_0 : Q3, τ_0_0 : Q4, τ_0_0.X : P3>
6971
protocol Q7: Q2, Q3, Q4 {
70-
associatedtype X: P3
72+
associatedtype X: P3 // expected-warning{{redeclaration of associated type 'X'}}
7173
}

‎test/Generics/protocol_type_aliases.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
func sameType<T>(_: T.Type, _: T.Type) {}
77

88
protocol P {
9-
associatedtype A
9+
associatedtype A // expected-note{{'A' declared here}}
1010
typealias X = A
1111
}
1212
protocol Q {
@@ -72,7 +72,7 @@ func useTypealias1<T: Q3_1>(_: T, _: T.T) {}
7272
// Subprotocols can force associated types in their parents to be concrete, and
7373
// this should be understood for types constrained by the subprotocols.
7474
protocol Q4: P {
75-
typealias A = Int
75+
typealias A = Int // expected-warning{{typealias overriding associated type 'A' from protocol 'P'}}
7676
}
7777
protocol Q5: P {
7878
typealias X = Int
@@ -98,13 +98,13 @@ func checkQ5_X<T: Q5>(x: T.Type) { sameType(getP_X(x), Int.self) }
9898
// Typealiases happen to allow imposing same type requirements between parent
9999
// protocols
100100
protocol P6_1 {
101-
associatedtype A
101+
associatedtype A // expected-note{{'A' declared here}}
102102
}
103103
protocol P6_2 {
104104
associatedtype B
105105
}
106106
protocol Q6: P6_1, P6_2 {
107-
typealias A = B
107+
typealias A = B // expected-warning{{typealias overriding associated type}}
108108
}
109109

110110
func getP6_1_A<T: P6_1>(_: T.Type) -> T.A.Type { return T.A.self }

0 commit comments

Comments
 (0)