Merge pull request #6346 from KingOfBrian/bugfix/SR-584

Warn if a non dynamic class declaration is overridden in an extension

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -1707,6 +1707,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", 
+      ())
+WARNING(override_class_declaration_in_extension_warning,none,
+      "cannot override a non-dynamic class declaration from an extension",
+      ())
 ERROR(override_throws,none,
       "cannot override non-throwing %select{method|initializer}0 with "
       "throwing %select{method|initializer}0", (bool))

lib/Sema/TypeCheckDecl.cpp

@@ -6087,6 +6087,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()) {
+        // 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)) {

test/Compatibility/override.swift

@@ -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}}
+}

test/decl/inherit/override.swift

@@ -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 {}
@@ -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 {
@@ -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}}