[vm] Remove non-covariant checks from closure bodies (part 1)

This change only affects compilation when running in non-precompiled
mode with --no-lazy-dispatchers enabled.

Instead of always compiling in non-covariant checks, even for closures
not called dynamically, remove the non-covariant checks from the closure
and instead do the non-covariant checks for dynamic calls during the
NoSuchMethodForCallStub fallback by calling
Function::DoArgumentTypesMatch.

Adds two overloads for Function::DoArgumentTypesMatch, one which takes a
function type argument vector and one which takes neither an
instantiator type argument vector or a function type argument vector.
For the versions that are not explicitly passed a type argument vector,
an appropriate one is constructed using the arguments. If there is not
enough information in the arguments, then we fall back to assuming the
empty type argument vector for the instantiator case and instantiating
to bounds in the function type argument case.

Fixes Function::DoArgumentTypesMatch to handle generic functions and to
check arguments appropriately according to the active null safety mode.
For generic functions, the provided or resulting function type vector
has non-covariant checks performed against the type parameter bounds.

This change uncovered one test that was incorrectly passing in strong
mode, see #42688 for details.

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-dartkb-linux-release-x64-try
Change-Id: I5030a46b356778448b51a243f16406eacf1c04bf
Bug: #40813
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153202
Commit-Queue: Tess Strickland <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: sstrickl

runtime/lib/mirrors.cc

@@ -1521,6 +1521,12 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 0, 5) {
                       InvocationMirror::kMethod);
     UNREACHABLE();
   }
+#if defined(DEBUG)
+  // Make sure the receiver is the null value, so that DoArgumentTypesMatch does
+  // not attempt to retrieve the instantiator type arguments from the receiver.
+  explicit_argument = args.At(args_descriptor.FirstArgIndex());
+  ASSERT(explicit_argument.IsNull());
+#endif
   const Object& type_error =
       Object::Handle(redirected_constructor.DoArgumentTypesMatch(
           args, args_descriptor, type_arguments));

runtime/vm/compiler/frontend/prologue_builder.cc

@@ -40,7 +40,8 @@ bool PrologueBuilder::PrologueSkippableOnUncheckedEntry(
 
 bool PrologueBuilder::HasEmptyPrologue(const Function& function) {
   return !function.HasOptionalParameters() && !function.IsGeneric() &&
-         !function.CanReceiveDynamicInvocation();
+         !function.CanReceiveDynamicInvocation() &&
+         !function.IsClosureFunction();
 }
 
 BlockEntryInstr* PrologueBuilder::BuildPrologue(BlockEntryInstr* entry,
@@ -81,6 +82,8 @@ BlockEntryInstr* PrologueBuilder::BuildPrologue(BlockEntryInstr* entry,
   }
 
   const bool is_empty_prologue = prologue.entry == prologue.current;
+  // Double-check we create empty prologues when HasEmptyPrologue returns true.
+  ASSERT(!HasEmptyPrologue(function_) || is_empty_prologue);
 
   // Always do this to preserve deoptid numbering.
   JoinEntryInstr* normal_code = BuildJoinEntry();

runtime/vm/compiler/frontend/scope_builder.cc

@@ -32,15 +32,7 @@ bool MethodCanSkipTypeChecksForNonCovariantArguments(
   // In AOT mode we don't dynamically generate such trampolines but instead rely
   // on a static analysis to discover which methods can be invoked dynamically,
   // and generate the necessary trampolines during precompilation.
-  if (method.name() == Symbols::Call().raw() ||
-      method.CanReceiveDynamicInvocation()) {
-    // Currently we consider all call methods to be invoked dynamically and
-    // don't mangle their names.
-    // TODO(vegorov) remove this once we also introduce special type checking
-    // entry point for closures.
-    return false;
-  }
-  return true;
+  return !method.CanReceiveDynamicInvocation();
 }
 
 ScopeBuilder::ScopeBuilder(ParsedFunction* parsed_function)

runtime/vm/dart_api_impl.cc

@@ -3806,11 +3806,14 @@ static Dart_Handle NewExternalTypedData(
   CHECK_LENGTH(length, ExternalTypedData::MaxElements(cid));
   Zone* zone = thread->zone();
   intptr_t bytes = length * ExternalTypedData::ElementSizeInBytes(cid);
-  const ExternalTypedData& result = ExternalTypedData::Handle(
-      zone,
-      ExternalTypedData::New(cid, reinterpret_cast<uint8_t*>(data), length,
-                             thread->heap()->SpaceForExternal(bytes)));
-  if (callback != NULL) {
+  auto& cls = Class::Handle(zone, thread->isolate()->class_table()->At(cid));
+  auto& result = Object::Handle(zone, cls.EnsureIsAllocateFinalized(thread));
+  if (result.IsError()) {
+    return Api::NewHandle(thread, result.raw());
+  }
+  result = ExternalTypedData::New(cid, reinterpret_cast<uint8_t*>(data), length,
+                                  thread->heap()->SpaceForExternal(bytes));
+  if (callback != nullptr) {
     AllocateFinalizableHandle(thread, result, peer, external_allocation_size,
                               callback);
   }

runtime/vm/dart_entry.cc

@@ -226,11 +226,20 @@ ObjectPtr DartEntry::ResolveCallable(const Array& arguments,
   // The null instance cannot resolve to a callable, so we can stop there.
   for (instance ^= arguments.At(receiver_index); !instance.IsNull();
        instance ^= arguments.At(receiver_index)) {
-    // If the current instance is a compatible callable, return its function.
-    if (instance.IsCallable(&function) &&
-        function.AreValidArgumentCounts(type_args_len, args_count,
-                                        named_args_count, nullptr)) {
-      return function.raw();
+    // The instance is a callable, so check that its function is compatible.
+    if (instance.IsCallable(&function)) {
+      bool matches = function.AreValidArgumentCounts(type_args_len, args_count,
+                                                     named_args_count, nullptr);
+
+      if (matches && type_args_len > 0 && function.IsClosureFunction()) {
+        // Though the closure function is generic, the closure itself may
+        // not be because it closes over delayed function type arguments.
+        matches = Closure::Cast(instance).IsGeneric(thread);
+      }
+
+      if (matches) {
+        return function.raw();
+      }
     }
 
     // Special case: closures are implemented with a call getter instead of a