Verify method overrides in checked mode only (fix issue 69).

runtime/vm/class_finalizer.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/class_finalizer.h"
 
 #include "vm/flags.h"
 #include "vm/heap.h"
 #include "vm/isolate.h"
 #include "vm/longjump.h"
@@ skipping 134 matching lines @@
       // Lookup function in 'cls' and all its super classes.
       Class& test_class = Class::Handle(cls.raw());
       Function& class_function =
           Function::Handle(test_class.LookupDynamicFunction(function_name));
       while (class_function.IsNull()) {
         test_class = test_class.SuperClass();
         if (test_class.IsNull()) break;
         class_function = test_class.LookupDynamicFunction(function_name);
       }
       if (class_function.IsNull()) {
+        const String& class_name = String::Handle(cls.Name());
         OS::Print("%s implements '%s' missing: '%s'\n",
-            cls.ToCString(),
+            class_name.ToCString(),
             interface_name.ToCString(),
             function_name.ToCString());
-      } else if (!class_function.IsAssignableTo(interface_function)) {
-        // TODO(regis): Shouldn't this be IsSubtypeOf instead of IsAssignableTo?
-        OS::Print("%s implements '%s' with wrong result type, wrong number of "
-                  "parameters, or wrong parameter types: '%s'\n",
-            cls.ToCString(),
-            interface_name.ToCString(),
-            function_name.ToCString());
+      } else if (!class_function.IsSubtypeOf(interface_function)) {
+        const String& class_name = String::Handle(cls.Name());
+        OS::Print("The type of the instance method '%s' in class '%s' is not a "

[hausner, 2011/10/17 22:56:26]

Can probably save a word:

The type of the instance ... -> The type of instance ...

[regis, 2011/10/17 22:59:13]

Done.

+                  "subtype of the type of '%s' in interface '%s'\n",
+                  function_name.ToCString(),
+                  class_name.ToCString(),
+                  function_name.ToCString(),
+                  interface_name.ToCString());
       }
     }
   }
 }
 #else
 
 void ClassFinalizer::VerifyClassImplements(const Class& cls) {}
 
 #endif
 
@@ skipping 637 matching lines @@
   visited->RemoveLast();
 }
 
 
 void ClassFinalizer::CheckForLegalOverrides(const Class& cls) {
   HANDLESCOPE();
   const Class& super = Class::Handle(cls.SuperClass());
   if (super.IsNull()) {
     return;
   }
-  // Check functions.
-  // TODO(regis): It is not clear from the spec that we should be checking this.
-  const Array& functions_array = Array::Handle(cls.functions());
-  Function& function = Function::Handle();
-  String& function_name = String::Handle();
-  intptr_t len = functions_array.Length();
-  for (intptr_t i = 0; i < len; i++) {
-    function ^= functions_array.At(i);
-    if (!function.is_static()) {
-      function_name ^= function.name();
-      Function& overridden_function =
-          Function::Handle(super.LookupDynamicFunction(function_name));
-      if (!overridden_function.IsNull() &&
-          !function.HasCompatibleParametersWith(overridden_function)) {
-        const String& class_name = String::Handle(cls.Name());
-        ReportError("class '%s' overrides function '%s' with incompatible "
-                    "parameters.\n",
-                    class_name.ToCString(), function_name.ToCString());
+  if (FLAG_enable_type_checks) {
+    // Check functions.
+    const Array& functions_array = Array::Handle(cls.functions());
+    Function& function = Function::Handle();
+    String& function_name = String::Handle();
+    const intptr_t len = functions_array.Length();
+    for (intptr_t i = 0; i < len; i++) {
+      function ^= functions_array.At(i);
+      if (!function.is_static()) {
+        function_name ^= function.name();
+        Function& overridden_function =
+            Function::Handle(super.LookupDynamicFunction(function_name));
+        if (!overridden_function.IsNull() &&
+            !function.IsSubtypeOf(overridden_function)) {
+          const String& class_name = String::Handle(cls.Name());
+          const String& super_class_name = String::Handle(
+              Class::Handle(overridden_function.owner()).Name());
+          ReportError("The type of the instance method '%s' in class '%s' is "

[hausner, 2011/10/17 22:56:26]

ditto

[regis, 2011/10/17 22:59:13]

Done.

+                      "not a subtype of the type of the overriden instance "
+                      "method '%s' in class '%s'\n",
+                      function_name.ToCString(),
+                      class_name.ToCString(),
+                      function_name.ToCString(),
+                      super_class_name.ToCString());
+        }
       }
-      // Function types are purposely not checked for assignability.
     }
   }
   // Check fields.
   const Array& fields_array = Array::Handle(cls.fields());
   Field& field = Field::Handle();
   String& field_name = String::Handle();
-  len = fields_array.Length();
+  const intptr_t len = fields_array.Length();
   for (intptr_t i = 0; i < len; i++) {
     field ^= fields_array.At(i);
     field_name ^= field.name();
     Field& super_field = Field::Handle(super.LookupStaticField(field_name));
     if (super_field.IsNull()) {
       super_field = super.LookupInstanceField(field_name);
     }
     if (!super_field.IsNull()) {
       // A static field may "override" a static field.
       if (!super_field.is_static() || !field.is_static()) {
@@ skipping 116 matching lines @@
   ASSERT(msg_buffer != NULL);
   va_list args;
   va_start(args, format);
   OS::VSNPrint(msg_buffer, kBufferLength, format, args);
   va_end(args);
   isolate->long_jump_base()->Jump(1, msg_buffer);
   UNREACHABLE();
 }
 
 }  // namespace dart