Report compile-time errors for conflicting overrides as specified by latest

runtime/vm/class_finalizer.cc

 // Copyright (c) 2013, 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 914 matching lines @@
     type = function.ParameterTypeAt(i);
     ResolveType(cls, type, kCanonicalize);
     type = FinalizeType(cls, type, kCanonicalize);
     // A malformed parameter type is mapped to dynamic.
     ASSERT(!type.IsMalformed());
     function.SetParameterTypeAt(i, type);
   }
 }
 
 
-// Check if an instance field or method of same name exists
+// Check if an instance field, getter, or method of same name exists
 // in any super class.
 static RawClass* FindSuperOwnerOfInstanceMember(const Class& cls,
-                                                const String& name) {
+                                                const String& name,
+                                                const String& getter_name) {
   Class& super_class = Class::Handle();
   Function& function = Function::Handle();
   Field& field = Field::Handle();
   super_class = cls.SuperClass();
   while (!super_class.IsNull()) {
     function = super_class.LookupFunction(name);
     if (!function.IsNull() && !function.is_static()) {
       return super_class.raw();
     }
+    function = super_class.LookupFunction(getter_name);
+    if (!function.IsNull() && !function.is_static()) {
+      return super_class.raw();
+    }
     field = super_class.LookupField(name);
     if (!field.IsNull() && !field.is_static()) {
       return super_class.raw();
     }
     super_class = super_class.SuperClass();
   }
   return Class::null();
 }
 
 
@@ skipping 54 matching lines @@
     bound = FinalizeType(cls, bound, kCanonicalize);
     type_param.set_bound(bound);
   }
 }
 
 
 void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
   // Note that getters and setters are explicitly listed as such in the list of
   // functions of a class, so we do not need to consider fields as implicitly
   // generating getters and setters.
-  // The only compile errors we report are therefore:
-  // - a getter having the same name as a method (but not a getter) in a super
-  //   class or in a subclass.
-  // - a static field, instance field, or static method (but not an instance
-  //   method) having the same name as an instance member in a super class.
+  // Most overriding conflicts are only static warnings, i.e. they are not
+  // reported as compile-time errors by the vm. However, signature conflicts in
+  // overrides can be reported if the flag --error_on_bad_override is specified.
+  // Static warning examples are:
+  // - a static getter 'v' conflicting with an inherited instance setter 'v='.
+  // - a static setter 'v=' conflicting with an inherited instance member 'v'.
+  // - an instance member 'v' conflicting with an accessible static member 'v'
+  //   or 'v=' of a super class (except that an instance method 'v' does not
+  //   conflict with an accessible static setter 'v=' of a super class).
+  // The compile-time errors we report are:
+  // - a static member 'v' conflicting with an inherited instance member 'v'.
+  // - a static setter 'v=' conflicting with an inherited instance setter 'v='.
+  // - an instance method conflicting with an inherited instance field or
+  //   instance getter.
+  // - an instance field or instance getter conflicting with an inherited
+  //   instance method.
 
   // Resolve type of fields and check for conflicts in super classes.
   Array& array = Array::Handle(cls.fields());
   Field& field = Field::Handle();
   AbstractType& type = AbstractType::Handle();
   String& name = String::Handle();
+  String& getter_name = String::Handle();
   Class& super_class = Class::Handle();
   const intptr_t num_fields = array.Length();
   for (intptr_t i = 0; i < num_fields; i++) {
     field ^= array.At(i);
     type = field.type();
     ResolveType(cls, type, kCanonicalize);
     type = FinalizeType(cls, type, kCanonicalize);
     field.set_type(type);
     name = field.name();
     if (field.is_static()) {
-      super_class = FindSuperOwnerOfInstanceMember(cls, name);
+      getter_name = Field::GetterSymbol(name);
+      super_class = FindSuperOwnerOfInstanceMember(cls, name, getter_name);
       if (!super_class.IsNull()) {
         const String& class_name = String::Handle(cls.Name());
         const String& super_class_name = String::Handle(super_class.Name());
         const Script& script = Script::Handle(cls.script());
         ReportError(Error::Handle(),  // No previous error.
                     script, field.token_pos(),
                     "static field '%s' of class '%s' conflicts with "
                     "instance member '%s' of super class '%s'",
                     name.ToCString(),
                     class_name.ToCString(),
@@ skipping 51 matching lines @@
                       type_name.ToCString());
         } else {
           // Do not report an error yet, even in checked mode, since the field
           // may not actually be used.
           // Also, we may be generating a snapshot in production mode that will
           // later be executed in checked mode, in which case an error needs to
           // be reported, should the field be accessed.
           // Therefore, we undo the optimization performed by the parser, i.e.
           // we create an implicit static final getter and reset the field value
           // to the sentinel value.
-          const String& getter_name = String::Handle(Field::GetterSymbol(name));
           const Function& getter = Function::Handle(
               Function::New(getter_name,
                             RawFunction::kImplicitStaticFinalGetter,
                             /* is_static = */ true,
                             /* is_const = */ field.is_const(),
                             /* is_abstract = */ false,
                             /* is_external = */ false,
                             cls,
                             field.token_pos()));
           getter.set_result_type(type);
@@ skipping 13 matching lines @@
     interfaces.Add(super_class);
     CollectInterfaces(super_class, interfaces);
     super_class = super_class.SuperClass();
   }
   // Resolve function signatures and check for conflicts in super classes and
   // interfaces.
   array = cls.functions();
   Function& function = Function::Handle();
   Function& overridden_function = Function::Handle();
   const intptr_t num_functions = array.Length();
-  String& function_name = String::Handle();
   Error& error = Error::Handle();
   for (intptr_t i = 0; i < num_functions; i++) {
     function ^= array.At(i);
     ResolveAndFinalizeSignature(cls, function);
-    function_name = function.name();
+    name = function.name();
+    if (FLAG_error_on_bad_override &&  // Report signature conflicts only.
+        !function.is_static() && !function.IsConstructor()) {
+      // A constructor cannot override anything.
+      for (int i = 0; i < interfaces.Length(); i++) {
+        super_class ^= interfaces.At(i);
+        overridden_function = super_class.LookupDynamicFunction(name);
+        if (!overridden_function.IsNull() &&
+            !function.HasCompatibleParametersWith(overridden_function,
+                                                  &error)) {
+          const String& class_name = String::Handle(cls.Name());
+          const String& super_class_name = String::Handle(super_class.Name());
+          const Script& script = Script::Handle(cls.script());
+          ReportError(error, script, function.token_pos(),
+                      "class '%s' overrides method '%s' of super class '%s' "
+                      "with incompatible parameters",
+                      class_name.ToCString(),
+                      name.ToCString(),
+                      super_class_name.ToCString());
+        }
+      }
+    }
+    if (function.IsSetterFunction()) {
+      if (function.is_static()) {
+        super_class = FindSuperOwnerOfFunction(cls, name);
+        if (!super_class.IsNull()) {
+          const String& class_name = String::Handle(cls.Name());
+          const String& super_class_name = String::Handle(super_class.Name());
+          const Script& script = Script::Handle(cls.script());
+          ReportError(Error::Handle(),  // No previous error.
+                      script, function.token_pos(),
+                      "static setter '%s' of class '%s' conflicts with "
+                      "instance setter '%s' of super class '%s'",
+                      name.ToCString(),
+                      class_name.ToCString(),
+                      name.ToCString(),
+                      super_class_name.ToCString());
+        }
+      }
+      continue;
+    }
+    if (function.IsGetterFunction()) {
+      getter_name = name.raw();
+      name = Field::NameFromGetter(getter_name);
+    } else {
+      getter_name = Field::GetterSymbol(name);
+    }
     if (function.is_static()) {
-      super_class = FindSuperOwnerOfInstanceMember(cls, function_name);
+      super_class = FindSuperOwnerOfInstanceMember(cls, name, getter_name);
       if (!super_class.IsNull()) {
         const String& class_name = String::Handle(cls.Name());
         const String& super_class_name = String::Handle(super_class.Name());
         const Script& script = Script::Handle(cls.script());
         ReportError(Error::Handle(),  // No previous error.
                     script, function.token_pos(),
-                    "static function '%s' of class '%s' conflicts with "
+                    "static method '%s' of class '%s' conflicts with "
                     "instance member '%s' of super class '%s'",
-                    function_name.ToCString(),
+                    name.ToCString(),
                     class_name.ToCString(),
-                    function_name.ToCString(),
+                    name.ToCString(),
                     super_class_name.ToCString());
       }
       // The function may be a still unresolved redirecting factory. Do not yet
       // try to resolve it in order to avoid cycles in class finalization.
-    } else if (FLAG_error_on_bad_override && !function.IsConstructor()) {
-      // A constructor cannot override anything.
-      for (int i = 0; i < interfaces.Length(); i++) {
-        super_class ^= interfaces.At(i);
-        overridden_function = super_class.LookupDynamicFunction(function_name);
-        if (!overridden_function.IsNull() &&
-            !function.HasCompatibleParametersWith(overridden_function,
-                                                  &error)) {
-          // Function types are purposely not checked for subtyping.
-          const String& class_name = String::Handle(cls.Name());
-          const String& super_class_name = String::Handle(super_class.Name());
-          const Script& script = Script::Handle(cls.script());
-          ReportError(error, script, function.token_pos(),
-                      "class '%s' overrides function '%s' of super class '%s' "
-                      "with incompatible parameters",
-                      class_name.ToCString(),
-                      function_name.ToCString(),
-                      super_class_name.ToCString());
-        }
-      }
-    }
-    if (function.IsGetterFunction()) {
-      name = Field::NameFromGetter(function_name);
+    } else if (function.IsGetterFunction()) {
       super_class = FindSuperOwnerOfFunction(cls, name);
       if (!super_class.IsNull()) {
         const String& class_name = String::Handle(cls.Name());
         const String& super_class_name = String::Handle(super_class.Name());
         const Script& script = Script::Handle(cls.script());
         ReportError(Error::Handle(),  // No previous error.
                     script, function.token_pos(),
                     "getter '%s' of class '%s' conflicts with "
-                    "function '%s' of super class '%s'",
+                    "method '%s' of super class '%s'",
                     name.ToCString(),
                     class_name.ToCString(),
                     name.ToCString(),
                     super_class_name.ToCString());
       }
     } else if (!function.IsSetterFunction()) {
       // A function cannot conflict with a setter, since they cannot
       // have the same name. Thus, we do not need to check setters.
-      name = Field::GetterName(function_name);
-      super_class = FindSuperOwnerOfFunction(cls, name);
+      super_class = FindSuperOwnerOfFunction(cls, getter_name);
       if (!super_class.IsNull()) {
         const String& class_name = String::Handle(cls.Name());
         const String& super_class_name = String::Handle(super_class.Name());
         const Script& script = Script::Handle(cls.script());
         ReportError(Error::Handle(),  // No previous error.
                     script, function.token_pos(),
-                    "function '%s' of class '%s' conflicts with "
+                    "method '%s' of class '%s' conflicts with "
                     "getter '%s' of super class '%s'",
-                    function_name.ToCString(),
+                    name.ToCString(),
                     class_name.ToCString(),
-                    function_name.ToCString(),
+                    name.ToCString(),
                     super_class_name.ToCString());
       }
     }
   }
 }
 
 
 // Copy the type parameters of the super and mixin classes to the
 // mixin application class. Change type arguments of super type and of
 // interfaces to refer to the respective type parameters of the mixin
@@ skipping 1010 matching lines @@
   expected_name ^= String::New("_offset");
   ASSERT(String::EqualsIgnoringPrivateKey(name, expected_name));
   field ^= fields_array.At(2);
   ASSERT(field.Offset() == TypedDataView::length_offset());
   name ^= field.name();
   ASSERT(name.Equals("length"));
 #endif
 }
 
 }  // namespace dart