Implement updated method overriding rules in the vm.

runtime/vm/object.cc

 // Copyright (c) 2012, 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/object.h"
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "vm/assembler.h"
 #include "vm/cpu.h"
@@ skipping 40 matching lines @@
 DEFINE_FLAG(int, huge_method_cutoff_in_tokens, 20000,
     "Huge method cutoff in tokens: Disables optimizations for huge methods.");
 DEFINE_FLAG(int, huge_method_cutoff_in_code_size, 200000,
     "Huge method cutoff in unoptimized code size (in bytes).");
 DEFINE_FLAG(bool, throw_on_javascript_int_overflow, false,
     "Throw an exception when the result of an integer calculation will not "
     "fit into a javascript integer.");
 DECLARE_FLAG(bool, trace_compiler);
 DECLARE_FLAG(bool, eliminate_type_checks);
 DECLARE_FLAG(bool, enable_type_checks);
+DECLARE_FLAG(bool, error_on_bad_override);
 
 static const char* kGetterPrefix = "get:";
 static const intptr_t kGetterPrefixLength = strlen(kGetterPrefix);
 static const char* kSetterPrefix = "set:";
 static const intptr_t kSetterPrefixLength = strlen(kSetterPrefix);
 
 cpp_vtable Object::handle_vtable_ = 0;
 cpp_vtable Object::builtin_vtables_[kNumPredefinedCids] = { 0 };
 cpp_vtable Smi::handle_vtable_ = 0;
 
@@ skipping 1829 matching lines @@
   // Prefinalized classes have a VM internal representation and no Dart fields.
   // Their instance size  is precomputed and field offsets are known.
   if (!is_prefinalized()) {
     // Compute offsets of instance fields and instance size.
     CalculateFieldOffsets();
   }
   set_is_finalized();
 }
 
 
-static const char* FormatPatchError(const char* format, const Object& obj) {
-  const char* msg = obj.ToCString();
-  intptr_t len = OS::SNPrint(NULL, 0, format, msg) + 1;
-  char* result = Isolate::Current()->current_zone()->Alloc<char>(len);
-  OS::SNPrint(result, len, format, msg);
-  return result;
+static RawError* FormatError(const Error& prev_error,
+                             const Script& script,
+                             intptr_t token_pos,
+                             const char* format, ...) {
+  va_list args;
+  va_start(args, format);
+  if (prev_error.IsNull()) {
+    return Parser::FormatError(script, token_pos, "Error", format, args);
+  } else {
+    return Parser::FormatErrorWithAppend(prev_error, script, token_pos,
+                                         "Error", format, args);
+  }
 }
 
 
 // Apply the members from the patch class to the original class.
-const char* Class::ApplyPatch(const Class& patch) const {
+bool Class::ApplyPatch(const Class& patch, Error* error) const {
+  ASSERT(error != NULL);
   ASSERT(!is_finalized());
   // Shared handles used during the iteration.
   String& member_name = String::Handle();
 
   const PatchClass& patch_class =
       PatchClass::Handle(PatchClass::New(*this, patch));
 
   Array& orig_list = Array::Handle(functions());
   intptr_t orig_len = orig_list.Length();
   Array& patch_list = Array::Handle(patch.functions());
@@ skipping 19 matching lines @@
     member_name ^= orig_func.name();
     func = patch.LookupFunction(member_name);
     if (func.IsNull()) {
       // Non-patched function is preserved, all patched functions are added in
       // the loop below.
       // However, an implicitly created constructor should not be preserved if
       // the patch provides a constructor or a factory. Wait for now.
       if (orig_func.raw() != orig_implicit_ctor.raw()) {
         new_functions.Add(orig_func);
       }
-    } else if (!func.HasCompatibleParametersWith(orig_func) &&
-               !(func.IsFactory() && orig_func.IsConstructor() &&
-                 (func.num_fixed_parameters() + 1 ==
-                  orig_func.num_fixed_parameters()))) {
-      return FormatPatchError("mismatched parameters: %s", member_name);
+    } else if (func.UserVisibleSignature() !=
+               orig_func.UserVisibleSignature()) {
+      // Compare user visible signatures to ignore different implicit parameters
+      // when patching a constructor with a factory.
+      *error = FormatError(*error,  // No previous error.
+                           Script::Handle(patch.script()), func.token_pos(),
+                           "signature mismatch: '%s'", member_name.ToCString());
+      return false;
     }
   }
   for (intptr_t i = 0; i < patch_len; i++) {
     func ^= patch_list.At(i);
     if (func.IsConstructor() || func.IsFactory()) {
       // Do not preserve the original implicit constructor, if any.
       orig_implicit_ctor = Function::null();
     }
     func.set_owner(patch_class);
     new_functions.Add(func);
@@ skipping 17 matching lines @@
   new_list = Array::New(patch_len + orig_len);
   for (intptr_t i = 0; i < patch_len; i++) {
     field ^= patch_list.At(i);
     field.set_owner(*this);
     member_name = field.name();
     // TODO(iposva): Verify non-public fields only.
 
     // Verify no duplicate additions.
     orig_field ^= LookupField(member_name);
     if (!orig_field.IsNull()) {
-      return FormatPatchError("duplicate field: %s", member_name);
+      *error = FormatError(*error,  // No previous error.
+                           Script::Handle(patch.script()), field.token_pos(),
+                           "duplicate field: %s", member_name.ToCString());
+      return false;
     }
     new_list.SetAt(i, field);
   }
   for (intptr_t i = 0; i < orig_len; i++) {
     field ^= orig_list.At(i);
     new_list.SetAt(patch_len + i, field);
   }
   SetFields(new_list);
 
   // The functions and fields in the patch class are no longer needed.
   patch.SetFunctions(Object::empty_array());
   patch.SetFields(Object::empty_array());
-  return NULL;
+  return true;
 }
 
 
 // Ensure that top level parsing of the class has been done.
 RawError* Class::EnsureIsFinalized(Isolate* isolate) const {
   // Finalized classes have already been parsed.
   if (is_finalized()) {
     return Error::null();
   }
   ASSERT(isolate != NULL);
@@ skipping 1087 matching lines @@
     }
     type_class = type.type_class();
     if (!type_class.IsDynamicClass()) {
       return false;
     }
   }
   return true;
 }
 
 
-static RawError* FormatError(const Error& prev_error,
-                             const Script& script,
-                             intptr_t token_pos,
-                             const char* format, ...) {
-  va_list args;
-  va_start(args, format);
-  if (prev_error.IsNull()) {
-    return Parser::FormatError(script, token_pos, "Error", format, args);
-  } else {
-    return Parser::FormatErrorWithAppend(prev_error, script, token_pos,
-                                         "Error", format, args);
-  }
-}
-
-
 bool AbstractTypeArguments::TypeTest(TypeTestKind test_kind,
                                      const AbstractTypeArguments& other,
                                      intptr_t len,
                                      Error* malformed_error) const {
   ASSERT(Length() >= len);
   ASSERT(!other.IsNull());
   ASSERT(other.Length() >= len);
   AbstractType& type = AbstractType::Handle();
   AbstractType& other_type = AbstractType::Handle();
   for (intptr_t i = 0; i < len; i++) {
@@ skipping 1199 matching lines @@
 }
 
 
 const char* Function::ToFullyQualifiedCString() const {
   char* chars = NULL;
   ConstructFunctionFullyQualifiedCString(*this, &chars, 0);
   return chars;
 }
 
 
-bool Function::HasCompatibleParametersWith(const Function& other) const {
-  const intptr_t num_fixed_params = num_fixed_parameters();
-  const intptr_t num_opt_pos_params = NumOptionalPositionalParameters();
-  const intptr_t other_num_fixed_params = other.num_fixed_parameters();
-  const intptr_t other_num_opt_pos_params =
-      other.NumOptionalPositionalParameters();
-  // A generative constructor may be compared to a redirecting factory and be
-  // compatible although it has an additional phase parameter.
-  const intptr_t num_ignored_params =
-      (other.IsRedirectingFactory() && IsConstructor()) ? 1 : 0;
-  // The default values of optional parameters can differ.
-  // This function requires the same arguments or less and accepts the same
-  // arguments or more.
-  if (((num_fixed_params - num_ignored_params) > other_num_fixed_params) ||
-      ((num_fixed_params - num_ignored_params) + num_opt_pos_params <
-       other_num_fixed_params + other_num_opt_pos_params)) {
+bool Function::HasCompatibleParametersWith(const Function& other,
+                                           Error* error) const {
+  ASSERT(FLAG_error_on_bad_override);
+  // Check that this function's signature type is a subtype of the other
+  // function's signature type.
+  if (!TypeTest(kIsSubtypeOf, Object::null_abstract_type_arguments(),
+                other, Object::null_abstract_type_arguments(), error)) {
+    // For more informative error reporting, use the location of the other
+    // function here, since the caller will use the location of this function.
+    *error = FormatError(
+        *error,  // A malformed error if non null.
+        Script::Handle(other.script()),
+        other.token_pos(),
+        "signature type '%s' of function '%s' is not a subtype of signature "
+        "type '%s' of function '%s'",
+        String::Handle(UserVisibleSignature()).ToCString(),
+        String::Handle(UserVisibleName()).ToCString(),
+        String::Handle(other.UserVisibleSignature()).ToCString(),
+        String::Handle(other.UserVisibleName()).ToCString());
     return false;
   }
+  // We should also check that if the other function explicitly specifies a
+  // default value for a formal parameter, this function does not specify a
+  // different default value for the same parameter. However, this check is not
+  // possible in the current implementation, because the default parameter
+  // values are not stored in the Function object, but discarded after a
+  // function is compiled.
   return true;
 }
 
 
 // If test_kind == kIsSubtypeOf, checks if the type of the specified parameter
 // of this function is a subtype or a supertype of the type of the specified
 // parameter of the other function.
 // If test_kind == kIsMoreSpecificThan, checks if the type of the specified
 // parameter of this function is more specific than the type of the specified
 // parameter of the other function.
@@ skipping 49 matching lines @@
   const intptr_t num_fixed_params = num_fixed_parameters();
   const intptr_t num_opt_pos_params = NumOptionalPositionalParameters();
   const intptr_t num_opt_named_params = NumOptionalNamedParameters();
   const intptr_t other_num_fixed_params = other.num_fixed_parameters();
   const intptr_t other_num_opt_pos_params =
       other.NumOptionalPositionalParameters();
   const intptr_t other_num_opt_named_params =
       other.NumOptionalNamedParameters();
   // This function requires the same arguments or less and accepts the same
   // arguments or more.
-  if ((num_fixed_params > other_num_fixed_params) ||
-      (num_fixed_params + num_opt_pos_params <
-       other_num_fixed_params + other_num_opt_pos_params) ||
+  // A generative constructor may be compared to a redirecting factory and be
+  // compatible although it has an additional phase parameter.
+  // More generally, we can ignore implicit parameters.
+  const intptr_t num_ignored_params = NumImplicitParameters();
+  const intptr_t other_num_ignored_params = other.NumImplicitParameters();
+  if (((num_fixed_params - num_ignored_params) >
+       (other_num_fixed_params - other_num_ignored_params)) ||
+      ((num_fixed_params - num_ignored_params + num_opt_pos_params) <
+       (other_num_fixed_params - other_num_ignored_params +
+        other_num_opt_pos_params)) ||
       (num_opt_named_params < other_num_opt_named_params)) {
     return false;
   }
   // Check the result type.
   AbstractType& other_res_type = AbstractType::Handle(other.result_type());
   if (!other_res_type.IsInstantiated()) {
     other_res_type = other_res_type.InstantiateFrom(other_type_arguments,
                                                     malformed_error);
     ASSERT((malformed_error == NULL) || malformed_error->IsNull());
   }
@@ skipping 12 matching lines @@
         return false;
       }
     } else {
       ASSERT(test_kind == kIsMoreSpecificThan);
       if (!res_type.IsMoreSpecificThan(other_res_type, malformed_error)) {
         return false;
       }
     }
   }
   // Check the types of fixed and optional positional parameters.
-  for (intptr_t i = 0;
-       i < other_num_fixed_params + other_num_opt_pos_params; i++) {
+  for (intptr_t i = 0; i < (other_num_fixed_params - other_num_ignored_params +
+                            other_num_opt_pos_params); i++) {
     if (!TestParameterType(test_kind,
-                           i, i, type_arguments, other, other_type_arguments,
+                           i + num_ignored_params, i + other_num_ignored_params,
+                           type_arguments, other, other_type_arguments,
                            malformed_error)) {
       return false;
     }
   }
   // Check the names and types of optional named parameters.
   if (other_num_opt_named_params == 0) {
     return true;
   }
   // Check that for each optional named parameter of type T of the other
   // function type, there exists an optional named parameter of this function
@@ skipping 10093 matching lines @@
 }
 
 
 void MirrorReference::PrintToJSONStream(JSONStream* stream, bool ref) const {
   stream->OpenObject();
   stream->CloseObject();
 }
 
 
 }  // namespace dart