[VM] Remove dead code associated with flag '--error-on-bad-override'

- Remove all dead code associated with flag '--error-on-bad-override' which is currently running only under !FLAG_strong which is not valid anymore
- Not removing the flag from flag_list for now as it is used in a number of tests and removing it from all these places would be a big change, it would be a NOP flag
- removed some dead code in 'loader.dart' which was invoked from the tag handler

Change-Id: I5474b51b4dee7471e80b4081e55b996f1d75612f
Reviewed-on: https://dart-review.googlesource.com/c/78382
Reviewed-by: Alexander Markov <alexmarkov@google.com>

runtime/bin/vmservice/loader.dart

@@ -992,26 +992,6 @@ _processLoadRequest(request) {
 
   // Handle the request specified in the tag.
   switch (tag) {
-    case _Dart_kScriptTag:
-      {
-        Uri uri = Uri.parse(request[4]);
-        // Remember the root script.
-        loaderState._rootScript = uri;
-        _handleResourceRequest(
-            loaderState, sp, traceLoading, tag, uri, uri, null);
-      }
-      break;
-    case _Dart_kSourceTag:
-    case _Dart_kImportTag:
-      {
-        // The url of the file being loaded.
-        var uri = Uri.parse(request[4]);
-        // The library that is importing/parting the file.
-        String libraryUrl = request[5];
-        _handleResourceRequest(
-            loaderState, sp, traceLoading, tag, uri, uri, libraryUrl);
-      }
-      break;
     case _Dart_kInitLoader:
       {
         String packageRoot = request[4];

runtime/include/dart_api.h

@@ -553,14 +553,13 @@ typedef struct {
  * for each part.
  */
 
-#define DART_FLAGS_CURRENT_VERSION (0x00000008)
+#define DART_FLAGS_CURRENT_VERSION (0x00000009)
 
 typedef struct {
   int32_t version;
   bool enable_type_checks;
   bool enable_asserts;
   bool enable_error_on_bad_type;
-  bool enable_error_on_bad_override;
   bool use_field_guards;
   bool use_osr;
   bool obfuscate;

runtime/vm/class_finalizer.cc

@@ -449,23 +449,6 @@ void ClassFinalizer::ResolveRedirectingFactoryTarget(
     return;
   }
 
-  if (!FLAG_strong && Isolate::Current()->error_on_bad_override()) {
-    // Verify that the target is compatible with the redirecting factory.
-    Error& error = Error::Handle();
-    if (!target.HasCompatibleParametersWith(factory, &error)) {
-      const Script& script = Script::Handle(target_class.script());
-      type = NewFinalizedMalformedType(
-          error, script, target.token_pos(),
-          "constructor '%s' has incompatible parameters with "
-          "redirecting factory '%s'",
-          String::Handle(target.name()).ToCString(),
-          String::Handle(factory.name()).ToCString());
-      factory.SetRedirectionType(type);
-      ASSERT(factory.RedirectionTarget() == Function::null());
-      return;
-    }
-  }
-
   // Verify that the target is const if the redirecting factory is const.
   if (factory.is_const() && !target.is_const()) {
     ReportError(target_class, target.token_pos(),
@@ -1561,9 +1544,7 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
   // functions of a class, so we do not need to consider fields as implicitly
   // generating getters and setters.
   // Most overriding conflicts are only static warnings, i.e. they are not
-  // reported as compile-time errors by the vm. However, in non-strong mode,
-  // signature conflicts in overrides can be reported if the flag
-  // --error_on_bad_override is specified.
+  // reported as compile-time errors by the vm.
   // 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'.
@@ -2532,15 +2513,6 @@ void ClassFinalizer::FinalizeClass(const Class& cls) {
   if (!super.IsNull()) {
     FinalizeClass(super);
   }
-  // Ensure interfaces are finalized in case we check for bad overrides.
-  Isolate* isolate = Isolate::Current();
-  if (isolate->error_on_bad_override() && !FLAG_strong) {
-    GrowableArray<const Class*> interfaces(4);
-    CollectInterfaces(cls, &interfaces);
-    for (intptr_t i = 0; i < interfaces.length(); i++) {
-      FinalizeClass(*interfaces.At(i));
-    }
-  }
   if (cls.IsMixinApplication()) {
     // Copy instance methods and fields from the mixin class.
     // This has to happen before the check whether the methods of

runtime/vm/dart.cc

@@ -718,8 +718,6 @@ const char* Dart::FeaturesString(Isolate* isolate,
     ADD_FLAG(asserts, enable_asserts, FLAG_enable_asserts);
     ADD_FLAG(error_on_bad_type, enable_error_on_bad_type,
              FLAG_error_on_bad_type);
-    ADD_FLAG(error_on_bad_override, enable_error_on_bad_override,
-             FLAG_error_on_bad_override);
     // sync-async and reify_generic_functions also affect deopt_ids.
     buffer.AddString(FLAG_sync_async ? " sync_async" : " no-sync_async");
     buffer.AddString(FLAG_reify_generic_functions

runtime/vm/dart_api_impl_test.cc

@@ -3593,7 +3593,6 @@ VM_UNIT_TEST_CASE(DartAPI_IsolateSetCheckedMode) {
   api_flags.enable_type_checks = true;
   api_flags.enable_asserts = true;
   api_flags.enable_error_on_bad_type = true;
-  api_flags.enable_error_on_bad_override = true;
   char* err;
   Dart_Isolate isolate =
       Dart_CreateIsolate(NULL, NULL, bin::core_isolate_snapshot_data,

runtime/vm/isolate.h

@@ -143,8 +143,6 @@ typedef FixedCache<intptr_t, CatchEntryMovesRefPtr, 16> CatchEntryMovesCache;
   V(NONPRODUCT, asserts, EnableAsserts, enable_asserts, FLAG_enable_asserts)   \
   V(NONPRODUCT, error_on_bad_type, ErrorOnBadType, enable_error_on_bad_type,   \
     FLAG_error_on_bad_type)                                                    \
-  V(NONPRODUCT, error_on_bad_override, ErrorOnBadOverride,                     \
-    enable_error_on_bad_override, FLAG_error_on_bad_override)                  \
   V(NONPRODUCT, use_field_guards, UseFieldGuards, use_field_guards,            \
     FLAG_use_field_guards)                                                     \
   V(NONPRODUCT, use_osr, UseOsr, use_osr, FLAG_use_osr)                        \

runtime/vm/kernel_isolate.cc

@@ -91,7 +91,6 @@ class RunKernelTask : public ThreadPool::Task {
     api_flags.enable_type_checks = false;
     api_flags.enable_asserts = false;
     api_flags.enable_error_on_bad_type = false;
-    api_flags.enable_error_on_bad_override = false;
     api_flags.unsafe_trust_strong_mode_types = false;
 #if !defined(DART_PRECOMPILER) && !defined(TARGET_ARCH_DBC)
     api_flags.use_field_guards = true;

runtime/vm/object.cc

@@ -7116,56 +7116,6 @@ const char* Function::ToQualifiedCString() const {
   return chars;
 }
 
-bool Function::HasCompatibleParametersWith(const Function& other,
-                                           Error* bound_error) const {
-  ASSERT(Isolate::Current()->error_on_bad_override());
-  ASSERT(!FLAG_strong);
-  ASSERT((bound_error != NULL) && bound_error->IsNull());
-  // Check that this function's signature type is a subtype of the other
-  // function's signature type.
-  // Map type parameters referred to by formal parameter types and result type
-  // in the signature to dynamic before the test.
-  // Note that type parameters declared by a generic signature are preserved.
-  Function& this_fun = Function::Handle(raw());
-  if (!this_fun.HasInstantiatedSignature(kCurrentClass)) {
-    this_fun = this_fun.InstantiateSignatureFrom(
-        Object::null_type_arguments(), Object::null_type_arguments(),
-        kNoneFree,  // Keep function type parameters, do not map to dynamic.
-        Heap::kOld);
-  }
-  Function& other_fun = Function::Handle(other.raw());
-  if (!other_fun.HasInstantiatedSignature(kCurrentClass)) {
-    other_fun = other_fun.InstantiateSignatureFrom(
-        Object::null_type_arguments(), Object::null_type_arguments(),
-        kNoneFree,  // Keep function type parameters, do not map to dynamic.
-        Heap::kOld);
-  }
-  if (!this_fun.TypeTest(kIsSubtypeOf, other_fun, bound_error, NULL,
-                         Heap::kOld)) {
-    // For more informative error reporting, use the location of the other
-    // function here, since the caller will use the location of this function.
-    auto space = Thread::Current()->IsMutatorThread() ? Heap::kNew : Heap::kOld;
-    *bound_error = LanguageError::NewFormatted(
-        *bound_error,  // A bound error if non null.
-        Script::Handle(other.script()), other.token_pos(), Report::AtLocation,
-        Report::kError, space,
-        "signature type '%s' of function '%s' is not a subtype of signature "
-        "type '%s' of function '%s'\n",
-        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;
-}
-
 RawFunction* Function::InstantiateSignatureFrom(
     const TypeArguments& instantiator_type_arguments,
     const TypeArguments& function_type_arguments,

runtime/vm/object.h

@@ -2699,12 +2699,6 @@ class Function : public Object {
 
   const char* ToQualifiedCString() const;
 
-  // Returns true if this function has parameters that are compatible with the
-  // parameters of the other function in order for this function to override the
-  // other function.
-  bool HasCompatibleParametersWith(const Function& other,
-                                   Error* bound_error) const;
-
   // Returns true if the type of this function is a subtype of the type of
   // the other function.
   bool IsSubtypeOf(const Function& other,