[vm] Support partial instantiation of noSuchMethod forwarders in VM.

This is complicated by the fact that the normal factory method for creating
Invocation objects references the arguments descriptor to determine how many
type arguments are passed. This doesn't work for partially instantiated
closures, because the arguments descriptor will say "0" type arguments even when
there may be delayed type arguments attached to the closure from partial
instantiation. The arguments descriptor can't be modified, so we have a new
factory method which takes the number of delayed type arguments directly.

Change-Id: Ic9a35a482b3b7ef80564e674cc6207873e255111
Reviewed-on: https://dart-review.googlesource.com/54245
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Régis Crelier <regis@google.com>

pkg/vm/lib/transformations/type_flow/entry_points.json

@@ -617,6 +617,12 @@
       "name": "_allocateInvocationMirror",
       "action": "call"
     },
+    {
+      "library": "dart:core",
+      "class": "_InvocationMirror",
+      "name": "_allocateInvocationMirrorForClosure",
+      "action": "call"
+    },
     {
       "library": "dart:core",
       "class": "_TypeError",

runtime/lib/invocation_mirror_patch.dart

@@ -41,6 +41,7 @@ class _InvocationMirror implements Invocation {
   final List _argumentsDescriptor;
   final List _arguments;
   final bool _isSuperInvocation;
+  final int _delayedTypeArgumentsLen;
 
   // External representation of the invocation mirror; populated on demand.
   Symbol _memberName;
@@ -79,16 +80,20 @@ class _InvocationMirror implements Invocation {
     return _memberName;
   }
 
+  int get _typeArgsLen {
+    int typeArgsLen = _argumentsDescriptor[_TYPE_ARGS_LEN];
+    return typeArgsLen == 0 ? _delayedTypeArgumentsLen : typeArgsLen;
+  }
+
   List<Type> get typeArguments {
     if (_typeArguments == null) {
-      int typeArgsLen = _argumentsDescriptor[_TYPE_ARGS_LEN];
-      if (typeArgsLen == 0) {
+      if (_typeArgsLen == 0) {
         return _typeArguments = const <Type>[];
       }
       // A TypeArguments object does not have a corresponding Dart class and
       // cannot be accessed as an array in Dart. Therefore, we need a native
       // call to unpack the individual types into a list.
-      _typeArguments = _unpackTypeArguments(_arguments[0], typeArgsLen);
+      _typeArguments = _unpackTypeArguments(_arguments[0], _typeArgsLen);
     }
     return _typeArguments;
   }
@@ -106,7 +111,7 @@ class _InvocationMirror implements Invocation {
         return _positionalArguments = const [];
       }
       // Exclude receiver and type args in the returned list.
-      int receiverIndex = _argumentsDescriptor[_TYPE_ARGS_LEN] > 0 ? 1 : 0;
+      int receiverIndex = _typeArgsLen > 0 ? 1 : 0;
       _positionalArguments = new _ImmutableList._from(
           _arguments, receiverIndex + 1, numPositionalArguments);
     }
@@ -121,7 +126,7 @@ class _InvocationMirror implements Invocation {
       if (numNamedArguments == 0) {
         return _namedArguments = const {};
       }
-      int receiverIndex = _argumentsDescriptor[_TYPE_ARGS_LEN] > 0 ? 1 : 0;
+      int receiverIndex = _typeArgsLen > 0 ? 1 : 0;
       _namedArguments = new Map<Symbol, dynamic>();
       for (int i = 0; i < numNamedArguments; i++) {
         int namedEntryIndex = _FIRST_NAMED_ENTRY + 2 * i;
@@ -164,10 +169,12 @@ class _InvocationMirror implements Invocation {
   }
 
   _InvocationMirror(this._functionName, this._argumentsDescriptor,
-      this._arguments, this._isSuperInvocation, this._type);
+      this._arguments, this._isSuperInvocation, this._type,
+      [this._delayedTypeArgumentsLen = 0]);
 
   _InvocationMirror._withoutType(this._functionName, this._typeArguments,
-      this._positionalArguments, this._namedArguments, this._isSuperInvocation);
+      this._positionalArguments, this._namedArguments, this._isSuperInvocation,
+      [this._delayedTypeArgumentsLen = 0]);
 
   static _allocateInvocationMirror(String functionName,
       List argumentsDescriptor, List arguments, bool isSuperInvocation,
@@ -175,4 +182,19 @@ class _InvocationMirror implements Invocation {
     return new _InvocationMirror(
         functionName, argumentsDescriptor, arguments, isSuperInvocation, type);
   }
+
+  // This factory is used when creating an `Invocation` for a closure call which
+  // may have delayed type arguments. In that case, the arguments descriptor will
+  // indicate 0 type arguments, but the actual number of type arguments are
+  // passed in `delayedTypeArgumentsLen`. If any type arguments are available,
+  // the type arguments vector will be the first entry in `arguments`.
+  static _allocateInvocationMirrorForClosure(
+      String functionName,
+      List argumentsDescriptor,
+      List arguments,
+      int type,
+      int delayedTypeArgumentsLen) {
+    return new _InvocationMirror(functionName, argumentsDescriptor, arguments,
+        false, type, delayedTypeArgumentsLen);
+  }
 }

runtime/vm/compiler/aot/precompiler.cc

@@ -510,6 +510,7 @@ static Dart_QualifiedFunctionName vm_entry_points[] = {
     {"dart:core", "_CastError", "_CastError._create"},
     {"dart:core", "_InternalError", "_InternalError."},
     {"dart:core", "_InvocationMirror", "_allocateInvocationMirror"},
+    {"dart:core", "_InvocationMirror", "_allocateInvocationMirrorForClosure"},
     {"dart:core", "_TypeError", "_TypeError._create"},
     {"dart:collection", "::", "_rehashObjects"},
     {"dart:isolate", "IsolateSpawnException", "IsolateSpawnException."},

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -5376,27 +5376,14 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
   body += StoreLocal(TokenPosition::kNoSource, argument_count_var);
   body += Drop();
   if (function.IsGeneric() && Isolate::Current()->reify_generic_functions()) {
-    Fragment test_generic;
-    JoinEntryInstr* join = BuildJoinEntry();
-
-    TargetEntryInstr *passed_type_args, *not_passed_type_args;
-    test_generic += B->LoadArgDescriptor();
-    test_generic += LoadField(ArgumentsDescriptor::type_args_len_offset());
-    test_generic += IntConstant(0);
-    test_generic += BranchIfEqual(&not_passed_type_args, &passed_type_args,
-                                  /*negate=*/false);
-
-    Fragment passed_type_args_frag(passed_type_args);
-    passed_type_args_frag += IntConstant(1);
-    passed_type_args_frag +=
-        StoreLocal(TokenPosition::kNoSource, argument_count_var);
-    passed_type_args_frag += Drop();
-    passed_type_args_frag += Goto(join);
-
-    Fragment not_passed_type_args_frag(not_passed_type_args);
-    not_passed_type_args_frag += Goto(join);
-
-    body += Fragment(test_generic.entry, join);
+    body += flow_graph_builder_->TestAnyTypeArgs(
+        [&]() {
+          return Fragment(
+              {IntConstant(1),
+               StoreLocal(TokenPosition::kNoSource, argument_count_var),
+               Drop()});
+        },
+        Fragment());
   }
 
   if (function.HasOptionalParameters()) {
@@ -5413,7 +5400,13 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
   //
   // var arguments = new Array<dynamic>(argument_count);
   //
-  // for (int i = 0; i < argument_count; ++i) {
+  // int i = 0;
+  // if (any type arguments are passed) {
+  //   arguments[0] = function_type_arguments;
+  //   ++i;
+  // }
+  //
+  // for (; i < argument_count; ++i) {
   //   arguments[i] = LoadFpRelativeSlot(
   //       kWordSize * (kParamEndSlotFromFp + argument_count - i));
   // }
@@ -5429,6 +5422,28 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
     body += StoreLocal(TokenPosition::kNoSource, index);
     body += Drop();
 
+    // if (any type arguments are passed) {
+    //   arguments[0] = function_type_arguments;
+    //   i = 1;
+    // }
+    if (function.IsGeneric() && Isolate::Current()->reify_generic_functions()) {
+      // clang-format off
+      std::function<Fragment()> store_type_arguments = [&]() {
+        return Fragment({
+            LoadLocal(arguments),
+            IntConstant(0),
+            LoadFunctionTypeArguments(),
+            StoreIndexed(kArrayCid),
+            Drop(),
+            IntConstant(1),
+            StoreLocal(TokenPosition::kNoSource, index),
+            Drop()
+          });
+      };
+      // clang-format on
+      body += B->TestAnyTypeArgs(store_type_arguments, Fragment());
+    }
+
     TargetEntryInstr* body_entry;
     TargetEntryInstr* loop_exit;
 
@@ -5515,10 +5530,6 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
   body += LoadLocal(arguments);
   body += PushArgument();
 
-  // false -> this is not super NSM
-  body += Constant(Bool::False());
-  body += PushArgument();
-
   if (throw_no_such_method_error) {
     const Function& parent =
         Function::ZoneHandle(Z, function.parent_function());
@@ -5541,12 +5552,30 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
   }
   body += PushArgument();
 
+  // Push the number of delayed type arguments.
+  if (function.IsClosureFunction()) {
+    LocalVariable* closure =
+        parsed_function()->node_sequence()->scope()->VariableAt(0);
+    body += B->TestDelayedTypeArgs(
+        closure,
+        Fragment({IntConstant(function.NumTypeParameters()),
+                  StoreLocal(TokenPosition::kNoSource, argument_count_var),
+                  Drop()}),
+        Fragment({IntConstant(0),
+                  StoreLocal(TokenPosition::kNoSource, argument_count_var),
+                  Drop()}));
+    body += LoadLocal(argument_count_var);
+  } else {
+    body += IntConstant(0);
+  }
+  body += PushArgument();
+
   const Class& mirror_class =
       Class::Handle(Z, Library::LookupCoreClass(Symbols::InvocationMirror()));
   ASSERT(!mirror_class.IsNull());
   const Function& allocation_function = Function::ZoneHandle(
-      Z, mirror_class.LookupStaticFunction(
-             Library::PrivateCoreLibName(Symbols::AllocateInvocationMirror())));
+      Z, mirror_class.LookupStaticFunction(Library::PrivateCoreLibName(
+             Symbols::AllocateInvocationMirrorForClosure())));
   ASSERT(!allocation_function.IsNull());
   body += StaticCall(TokenPosition::kMinSource, allocation_function,
                      /* argument_count = */ 5, ICData::kStatic);

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -1385,6 +1385,19 @@ Fragment BaseFlowGraphBuilder::TestDelayedTypeArgs(LocalVariable* closure,
   return Fragment(test.entry, join);
 }
 
+Fragment BaseFlowGraphBuilder::TestAnyTypeArgs(
+    std::function<Fragment()> present,
+    Fragment absent) {
+  if (parsed_function_->function().IsClosureFunction()) {
+    LocalVariable* closure =
+        parsed_function_->node_sequence()->scope()->VariableAt(0);
+    return TestTypeArgsLen(TestDelayedTypeArgs(closure, present(), absent),
+                           present(), 0);
+  } else {
+    return TestTypeArgsLen(absent, present(), 0);
+  }
+}
+
 Fragment FlowGraphBuilder::RethrowException(TokenPosition position,
                                             int catch_try_index) {
   Fragment instructions;

runtime/vm/compiler/frontend/kernel_to_il.h

@@ -7,6 +7,7 @@
 
 #if !defined(DART_PRECOMPILED_RUNTIME)
 
+#include <functional>
 #include <initializer_list>
 
 #include "vm/growable_array.h"
@@ -647,6 +648,7 @@ class BaseFlowGraphBuilder {
   Fragment TestDelayedTypeArgs(LocalVariable* closure,
                                Fragment present,
                                Fragment absent);
+  Fragment TestAnyTypeArgs(std::function<Fragment()> present, Fragment absent);
 
   JoinEntryInstr* BuildThrowNoSuchMethod();
 

runtime/vm/symbols.h

@@ -325,6 +325,7 @@ class ObjectPointerVisitor;
   V(ForwardingCorpse, "ForwardingCorpse")                                      \
   V(InvocationMirror, "_InvocationMirror")                                     \
   V(AllocateInvocationMirror, "_allocateInvocationMirror")                     \
+  V(AllocateInvocationMirrorForClosure, "_allocateInvocationMirrorForClosure") \
   V(toString, "toString")                                                      \
   V(_lookupHandler, "_lookupHandler")                                          \
   V(_handleMessage, "_handleMessage")                                          \

tests/language_2/language_2_dart2js.status

@@ -1958,6 +1958,7 @@ wrong_number_type_arguments_test/none: Pass
 const_constructor3_test/04: MissingCompileTimeError # OK - Subtype check uses JS number semantics.
 covariant_subtyping_test: Crash
 ct_const_test: RuntimeError
+nosuchmethod_forwarding/nosuchmethod_forwarding_partial_instantiation_test: RuntimeError
 
 [ $compiler == dart2js && $fasta && !$strong ]
 *: SkipByDesign

tests/language_2/language_2_vm.status

@@ -1323,6 +1323,8 @@ mixin_illegal_superclass_test: Skip # Issues 24478 and 23773
 mixin_mixin6_test: RuntimeError, OK # Not running in strong mode.
 multiline_strings_test: Fail # Issue 23020
 no_main_test/01: Skip
+nosuchmethod_forwarding/nosuchmethod_forwarding_partial_instantiation: RuntimeError # ByDesign
+nosuchmethod_forwarding/nosuchmethod_forwarding_partial_instantiation_test: RuntimeError
 regress_19413_test: MissingCompileTimeError
 regress_21793_test/01: MissingCompileTimeError
 super_test: Fail, OK

tests/language_2/nosuchmethod_forwarding/nosuchmethod_forwarding_partial_instantiation_test.dart

@@ -0,0 +1,25 @@
+// Copyright (c) 2018, 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.
+
+// Testing that `noSuchMethod` forwarders can be partially instantiated, and
+// that their delayed type arguments are transparently passed to `noSuchMethod`.
+
+import 'package:expect/expect.dart';
+
+class C {
+  T test1<T>(T x);
+
+  dynamic noSuchMethod(Invocation invoke) {
+    Expect.equals(invoke.typeArguments.length, 1);
+    Expect.equals(invoke.typeArguments[0].toString(), 'int');
+    Expect.equals(invoke.positionalArguments[0], 1);
+    return null;
+  }
+}
+
+void main() {
+  var c = new C();
+  int Function(int) k1 = c.test1;
+  k1(1);
+}