Skip to content

Override checking: believe full-name matches over types. #2466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4789,6 +4789,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
enum class OverrideCheckingAttempt {
PerfectMatch,
MismatchedOptional,
MismatchedTypes,
BaseName,
BaseNameWithMismatchedOptional,
Final
Expand Down Expand Up @@ -4820,6 +4821,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
decl->getFullName());
break;
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedTypes:
case OverrideCheckingAttempt::BaseNameWithMismatchedOptional:
if (isa<ConstructorDecl>(decl))
TC.diagnose(decl, diag::initializer_does_not_override);
Expand Down Expand Up @@ -4910,6 +4912,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
auto superclassMetaTy = MetatypeType::get(superclass);
DeclName name = decl->getFullName();
bool hadExactMatch = false;
LookupResult members;

do {
switch (attempt) {
Expand All @@ -4920,12 +4923,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
if (!decl->getAttrs().hasAttribute<OverrideAttr>())
return false;
break;
case OverrideCheckingAttempt::MismatchedTypes:
break;
case OverrideCheckingAttempt::BaseName:
// Don't keep looking if this is already a simple name, or if there
// are no arguments.
if (name.isSimpleName() || name.getArgumentNames().empty())
return false;
name = name.getBaseName();
members.clear();
break;
case OverrideCheckingAttempt::BaseNameWithMismatchedOptional:
break;
Expand All @@ -4934,11 +4940,12 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
return false;
}

NameLookupOptions lookupOptions
= defaultMemberLookupOptions - NameLookupFlags::DynamicLookup;
LookupResult members = TC.lookupMember(decl->getDeclContext(),
superclassMetaTy, name,
lookupOptions);
if (members.empty()) {
NameLookupOptions lookupOptions =
defaultMemberLookupOptions - NameLookupFlags::DynamicLookup;
members = TC.lookupMember(decl->getDeclContext(), superclassMetaTy,
name, lookupOptions);
}

for (auto memberResult : members) {
auto member = memberResult.Decl;
Expand Down Expand Up @@ -5015,7 +5022,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
// If this is a property, we accept the match and then reject it below
// if the types don't line up, since you can't overload properties based
// on types.
if (isa<VarDecl>(parentDecl)) {
if (isa<VarDecl>(parentDecl) ||
attempt == OverrideCheckingAttempt::MismatchedTypes) {
matches.push_back({parentDecl, false, parentDeclTy});
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion test/attr/attr_autoclosure.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func func11(_: @autoclosure(escaping) @noescape () -> ()) { } // expected-error{

class Super {
func f1(_ x: @autoclosure(escaping) () -> ()) { }
func f2(_ x: @autoclosure(escaping) () -> ()) { }
func f2(_ x: @autoclosure(escaping) () -> ()) { } // expected-note {{potential overridden instance method 'f2' here}}
func f3(x: @autoclosure () -> ()) { }
}

Expand Down
8 changes: 4 additions & 4 deletions test/attr/attr_override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class A {
var v8: Int = 0 // expected-note {{attempt to override property here}}
var v9: Int { return 5 } // expected-note{{attempt to override property here}}

subscript (i: Int) -> String {
subscript (i: Int) -> String { // expected-note{{potential overridden subscript 'subscript' here}}
get {
return "hello"
}
Expand All @@ -48,7 +48,7 @@ class A {
}
}

subscript (d: Double) -> String { // expected-note{{overridden declaration is here}}
subscript (d: Double) -> String { // expected-note{{overridden declaration is here}} expected-note{{potential overridden subscript 'subscript' here}}
get {
return "hello"
}
Expand All @@ -57,11 +57,11 @@ class A {
}
}

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

subscript (i: Int16) -> A { // expected-note{{attempt to override subscript here}}
subscript (i: Int16) -> A { // expected-note{{attempt to override subscript here}} expected-note{{potential overridden subscript 'subscript' here}}
get { return self }
set { }
}
Expand Down
25 changes: 23 additions & 2 deletions test/decl/class/override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,19 @@ class G {

func g1(_: Int, string: String) { } // expected-note{{potential overridden instance method 'g1(_:string:)' here}} {{28-28=string }}
func g1(_: Int, path: String) { } // expected-note{{potential overridden instance method 'g1(_:path:)' here}} {{28-28=path }}

func g2(_: Int, string: String) { } // expected-note{{potential overridden instance method 'g2(_:string:)' here}} {{none}}
func g2(_: Int, path: String) { }

func g3(_: Int, _ another: Int) { }
func g3(_: Int, path: String) { } // expected-note{{potential overridden instance method 'g3(_:path:)' here}} {{none}}

func g4(_: Int, _ another: Int) { }
func g4(_: Int, path: String) { }

init(a: Int) {} // expected-note {{potential overridden initializer 'init(a:)' here}} {{none}}
init(a: String) {} // expected-note {{potential overridden initializer 'init(a:)' here}} {{17-17=a }} expected-note {{potential overridden initializer 'init(a:)' here}} {{none}}
init(b: String) {} // expected-note {{potential overridden initializer 'init(b:)' here}} {{17-17=b }} expected-note {{potential overridden initializer 'init(b:)' here}} {{none}}
}

class H : G {
Expand All @@ -136,7 +149,15 @@ class H : G {

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

override func g1(_: Int, s: String) { } // expected-error{{declaration 'g1(_:s:)' has different argument names from any potential overrides}}
override func g1(_: Int, s: String) { } // expected-error{{declaration 'g1(_:s:)' has different argument names from any potential overrides}}{{none}}
override func g2(_: Int, string: Int) { } // expected-error{{method does not override any method from its superclass}} {{none}}
override func g3(_: Int, path: Int) { } // expected-error{{method does not override any method from its superclass}} {{none}}
override func g4(_: Int, string: Int) { } // expected-error{{argument names for method 'g4(_:string:)' do not match those of overridden method 'g4'}} {{28-28=_ }}

override init(x: Int) {} // expected-error{{argument names for initializer 'init(x:)' do not match those of overridden initializer 'init(a:)'}} {{17-17=a }}
override init(x: String) {} // expected-error{{declaration 'init(x:)' has different argument names from any potential overrides}} {{none}}
override init(a: Double) {} // expected-error{{initializer does not override a designated initializer from its superclass}} {{none}}
override init(b: Double) {} // expected-error{{initializer does not override a designated initializer from its superclass}} {{none}}
}

@objc class IUOTestBaseClass {
Expand Down Expand Up @@ -221,7 +242,7 @@ class GenericBase<T> {}
class ConcreteDerived: GenericBase<Int> {}

class OverriddenWithConcreteDerived<T> {
func foo() -> GenericBase<T> {}
func foo() -> GenericBase<T> {} // expected-note{{potential overridden instance method 'foo()' here}}
}
class OverridesWithMismatchedConcreteDerived<T>:
OverriddenWithConcreteDerived<T> {
Expand Down