Skip to content

Commit bd18590

Browse files
committed
Override checking: believe full-name matches over types. (#2466)
This at least emits notes when someone overrides something but gets the types a little wrong (more than just mismatched optionals, as handled in d669d15). Part of rdar://problem/26183575
1 parent 1b495b4 commit bd18590

File tree

4 files changed

+42
-13
lines changed

4 files changed

+42
-13
lines changed

lib/Sema/TypeCheckDecl.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4789,6 +4789,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
47894789
enum class OverrideCheckingAttempt {
47904790
PerfectMatch,
47914791
MismatchedOptional,
4792+
MismatchedTypes,
47924793
BaseName,
47934794
BaseNameWithMismatchedOptional,
47944795
Final
@@ -4820,6 +4821,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
48204821
decl->getFullName());
48214822
break;
48224823
case OverrideCheckingAttempt::MismatchedOptional:
4824+
case OverrideCheckingAttempt::MismatchedTypes:
48234825
case OverrideCheckingAttempt::BaseNameWithMismatchedOptional:
48244826
if (isa<ConstructorDecl>(decl))
48254827
TC.diagnose(decl, diag::initializer_does_not_override);
@@ -4910,6 +4912,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
49104912
auto superclassMetaTy = MetatypeType::get(superclass);
49114913
DeclName name = decl->getFullName();
49124914
bool hadExactMatch = false;
4915+
LookupResult members;
49134916

49144917
do {
49154918
switch (attempt) {
@@ -4920,12 +4923,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
49204923
if (!decl->getAttrs().hasAttribute<OverrideAttr>())
49214924
return false;
49224925
break;
4926+
case OverrideCheckingAttempt::MismatchedTypes:
4927+
break;
49234928
case OverrideCheckingAttempt::BaseName:
49244929
// Don't keep looking if this is already a simple name, or if there
49254930
// are no arguments.
49264931
if (name.isSimpleName() || name.getArgumentNames().empty())
49274932
return false;
49284933
name = name.getBaseName();
4934+
members.clear();
49294935
break;
49304936
case OverrideCheckingAttempt::BaseNameWithMismatchedOptional:
49314937
break;
@@ -4934,11 +4940,12 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
49344940
return false;
49354941
}
49364942

4937-
NameLookupOptions lookupOptions
4938-
= defaultMemberLookupOptions - NameLookupFlags::DynamicLookup;
4939-
LookupResult members = TC.lookupMember(decl->getDeclContext(),
4940-
superclassMetaTy, name,
4941-
lookupOptions);
4943+
if (members.empty()) {
4944+
NameLookupOptions lookupOptions =
4945+
defaultMemberLookupOptions - NameLookupFlags::DynamicLookup;
4946+
members = TC.lookupMember(decl->getDeclContext(), superclassMetaTy,
4947+
name, lookupOptions);
4948+
}
49424949

49434950
for (auto memberResult : members) {
49444951
auto member = memberResult.Decl;
@@ -5015,7 +5022,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
50155022
// If this is a property, we accept the match and then reject it below
50165023
// if the types don't line up, since you can't overload properties based
50175024
// on types.
5018-
if (isa<VarDecl>(parentDecl)) {
5025+
if (isa<VarDecl>(parentDecl) ||
5026+
attempt == OverrideCheckingAttempt::MismatchedTypes) {
50195027
matches.push_back({parentDecl, false, parentDeclTy});
50205028
continue;
50215029
}

test/attr/attr_autoclosure.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func func11(_: @autoclosure(escaping) @noescape () -> ()) { } // expected-error{
7777

7878
class Super {
7979
func f1(_ x: @autoclosure(escaping) () -> ()) { }
80-
func f2(_ x: @autoclosure(escaping) () -> ()) { }
80+
func f2(_ x: @autoclosure(escaping) () -> ()) { } // expected-note {{potential overridden instance method 'f2' here}}
8181
func f3(x: @autoclosure () -> ()) { }
8282
}
8383

test/attr/attr_override.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class A {
3939
var v8: Int = 0 // expected-note {{attempt to override property here}}
4040
var v9: Int { return 5 } // expected-note{{attempt to override property here}}
4141

42-
subscript (i: Int) -> String {
42+
subscript (i: Int) -> String { // expected-note{{potential overridden subscript 'subscript' here}}
4343
get {
4444
return "hello"
4545
}
@@ -48,7 +48,7 @@ class A {
4848
}
4949
}
5050

51-
subscript (d: Double) -> String { // expected-note{{overridden declaration is here}}
51+
subscript (d: Double) -> String { // expected-note{{overridden declaration is here}} expected-note{{potential overridden subscript 'subscript' here}}
5252
get {
5353
return "hello"
5454
}
@@ -57,11 +57,11 @@ class A {
5757
}
5858
}
5959

60-
subscript (i: Int8) -> A {
60+
subscript (i: Int8) -> A { // expected-note{{potential overridden subscript 'subscript' here}}
6161
get { return self }
6262
}
6363

64-
subscript (i: Int16) -> A { // expected-note{{attempt to override subscript here}}
64+
subscript (i: Int16) -> A { // expected-note{{attempt to override subscript here}} expected-note{{potential overridden subscript 'subscript' here}}
6565
get { return self }
6666
set { }
6767
}

test/decl/class/override.swift

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,19 @@ class G {
124124

125125
func g1(_: Int, string: String) { } // expected-note{{potential overridden instance method 'g1(_:string:)' here}} {{28-28=string }}
126126
func g1(_: Int, path: String) { } // expected-note{{potential overridden instance method 'g1(_:path:)' here}} {{28-28=path }}
127+
128+
func g2(_: Int, string: String) { } // expected-note{{potential overridden instance method 'g2(_:string:)' here}} {{none}}
129+
func g2(_: Int, path: String) { }
130+
131+
func g3(_: Int, _ another: Int) { }
132+
func g3(_: Int, path: String) { } // expected-note{{potential overridden instance method 'g3(_:path:)' here}} {{none}}
133+
134+
func g4(_: Int, _ another: Int) { }
135+
func g4(_: Int, path: String) { }
136+
137+
init(a: Int) {} // expected-note {{potential overridden initializer 'init(a:)' here}} {{none}}
138+
init(a: String) {} // expected-note {{potential overridden initializer 'init(a:)' here}} {{17-17=a }} expected-note {{potential overridden initializer 'init(a:)' here}} {{none}}
139+
init(b: String) {} // expected-note {{potential overridden initializer 'init(b:)' here}} {{17-17=b }} expected-note {{potential overridden initializer 'init(b:)' here}} {{none}}
127140
}
128141

129142
class H : G {
@@ -136,7 +149,15 @@ class H : G {
136149

137150
override func f7(_: Int, int value: Int) { } // okay
138151

139-
override func g1(_: Int, s: String) { } // expected-error{{declaration 'g1(_:s:)' has different argument names from any potential overrides}}
152+
override func g1(_: Int, s: String) { } // expected-error{{declaration 'g1(_:s:)' has different argument names from any potential overrides}}{{none}}
153+
override func g2(_: Int, string: Int) { } // expected-error{{method does not override any method from its superclass}} {{none}}
154+
override func g3(_: Int, path: Int) { } // expected-error{{method does not override any method from its superclass}} {{none}}
155+
override func g4(_: Int, string: Int) { } // expected-error{{argument names for method 'g4(_:string:)' do not match those of overridden method 'g4'}} {{28-28=_ }}
156+
157+
override init(x: Int) {} // expected-error{{argument names for initializer 'init(x:)' do not match those of overridden initializer 'init(a:)'}} {{17-17=a }}
158+
override init(x: String) {} // expected-error{{declaration 'init(x:)' has different argument names from any potential overrides}} {{none}}
159+
override init(a: Double) {} // expected-error{{initializer does not override a designated initializer from its superclass}} {{none}}
160+
override init(b: Double) {} // expected-error{{initializer does not override a designated initializer from its superclass}} {{none}}
140161
}
141162

142163
@objc class IUOTestBaseClass {
@@ -221,7 +242,7 @@ class GenericBase<T> {}
221242
class ConcreteDerived: GenericBase<Int> {}
222243

223244
class OverriddenWithConcreteDerived<T> {
224-
func foo() -> GenericBase<T> {}
245+
func foo() -> GenericBase<T> {} // expected-note{{potential overridden instance method 'foo()' here}}
225246
}
226247
class OverridesWithMismatchedConcreteDerived<T>:
227248
OverriddenWithConcreteDerived<T> {

0 commit comments

Comments
 (0)