Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,12 @@ ERROR(override_ownership_mismatch,none,
"cannot override %select{strong|weak|unowned|unowned(unsafe)}0 property "
"with %select{strong|weak|unowned|unowned(unsafe)}1 property",
(/*Ownership*/unsigned, /*Ownership*/unsigned))
ERROR(override_class_declaration_in_extension,none,
"cannot override a non-dynamic class declaration from an extension.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: remove the .

())
WARNING(override_class_declaration_in_extension_warning,none,
"cannot override a non-dynamic class declaration from an extension.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

())
ERROR(override_throws,none,
"cannot override non-throwing %select{method|initializer}0 with "
"throwing %select{method|initializer}0", (bool))
Expand Down
17 changes: 17 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6072,6 +6072,23 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
new (TC.Context) OverrideAttr(SourceLoc()));
}

// If the overridden method is declared in a Swift Class Declaration,
// dispatch will use table dispatch. If the override is in an extension
// warn, since it is not added to the class vtable.
//
// FIXME: Only warn if the extension is in another module, and if
// it is in the same module, update the vtable.
if (auto *baseDecl = dyn_cast<ClassDecl>(base->getDeclContext())) {
if (baseDecl->hasKnownSwiftImplementation() &&
!base->isDynamic() &&
override->getDeclContext()->isExtensionContext()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this conditional on !TC.Context.isSwiftVersion3(), or turn it into a warning?

To test both the Swift 3 and 4 behavior, have test/decl/inherit/override.swift pass -swift-version 4 in its RUN line, and add a new test/Compatibility/override.swift which tests the Swift 3 behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh thanks, I did not consider compatibility. I should be able to get a fix up soon.

// For compatibility, only generate a warning in Swift 3
TC.diagnose(override, (TC.Context.isSwiftVersion3()
? diag::override_class_declaration_in_extension_warning
: diag::override_class_declaration_in_extension));
TC.diagnose(base, diag::overridden_here);
}
}
// If the overriding declaration is 'throws' but the base is not,
// complain.
if (auto overrideFn = dyn_cast<AbstractFunctionDecl>(override)) {
Expand Down
11 changes: 11 additions & 0 deletions test/Compatibility/override.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %target-typecheck-verify-swift -parse-as-library -swift-version 3

class A {
@objc func objcVirtualFunction() { } // expected-note{{overridden declaration is here}}
}

class B : A { }

extension B {
override func objcVirtualFunction() { } // expected-warning{{cannot override a non-dynamic class declaration from an extension}}
}
16 changes: 11 additions & 5 deletions test/decl/inherit/override.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -parse-as-library
// RUN: %target-typecheck-verify-swift -parse-as-library -swift-version 4

@objc class ObjCClassA {}
@objc class ObjCClassB : ObjCClassA {}
Expand All @@ -7,8 +7,11 @@ class A {
func f1() { } // expected-note{{overridden declaration is here}}
func f2() -> A { } // expected-note{{overridden declaration is here}}

@objc func f3() { }
@objc func f4() -> ObjCClassA { }
@objc func f3() { } // expected-note{{overridden declaration is here}}
@objc func f4() -> ObjCClassA { } // expected-note{{overridden declaration is here}}

dynamic func f3D() { }
dynamic func f4D() -> ObjCClassA { }
}

extension A {
Expand All @@ -25,8 +28,11 @@ extension B {
func f1() { } // expected-error{{declarations in extensions cannot override yet}}
func f2() -> B { } // expected-error{{declarations in extensions cannot override yet}}

override func f3() { }
override func f4() -> ObjCClassB { }
override func f3() { } // expected-error{{cannot override a non-dynamic class declaration from an extension}}
override func f4() -> ObjCClassB { } // expected-error{{cannot override a non-dynamic class declaration from an extension}}

override func f3D() { }
override func f4D() -> ObjCClassB { }

func f5() { } // expected-error{{declarations in extensions cannot override yet}}
func f6() -> A { } // expected-error{{declarations in extensions cannot override yet}}
Expand Down