From 772c9bb5f33fcbd6c126bd4fda2bef850fea40e4 Mon Sep 17 00:00:00 2001 From: Samir Jindel Date: Sat, 5 May 2018 21:26:37 +0000 Subject: [PATCH] [vm/kernel] Re-land partial instantiation of local functions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change has been adjusted to account for the recent fix to Internal_prependTypeArguments. Reviewed-on: https://dart-review.googlesource.com/50980 Commit-Queue: Samir Jindel Reviewed-by: Régis Crelier Change-Id: I434a42578b91f0930b088c4d264d882c3ef5b4a3 Reviewed-on: https://dart-review.googlesource.com/53801 --- runtime/lib/object.cc | 24 +------- .../frontend/kernel_binary_flowgraph.cc | 58 +------------------ runtime/vm/compiler/frontend/kernel_to_il.cc | 55 ++++++++++++++++++ runtime/vm/compiler/frontend/kernel_to_il.h | 17 ++++++ .../vm/compiler/frontend/prologue_builder.cc | 47 +++++++++------ .../vm/compiler/frontend/prologue_builder.h | 2 +- runtime/vm/object.cc | 29 ++++++++-- runtime/vm/object.h | 5 ++ .../language_2/instantiate_tearoff_test.dart | 36 +++++++++--- tests/language_2/language_2_kernel.status | 2 - tests/language_2/language_2_vm.status | 1 + .../vm/regress_33040_instantiation_test.dart | 35 +++++++++++ 12 files changed, 202 insertions(+), 109 deletions(-) create mode 100644 tests/language_2/vm/regress_33040_instantiation_test.dart diff --git a/runtime/lib/object.cc b/runtime/lib/object.cc index f47a80a7901..9008f082972 100644 --- a/runtime/lib/object.cc +++ b/runtime/lib/object.cc @@ -462,28 +462,10 @@ DEFINE_NATIVE_ENTRY(Internal_prependTypeArguments, 4) { TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)); const TypeArguments& parent_type_arguments = TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(1)); - if (function_type_arguments.IsNull() && parent_type_arguments.IsNull()) { - return TypeArguments::null(); - } GET_NON_NULL_NATIVE_ARGUMENT(Smi, smi_parent_len, arguments->NativeArgAt(2)); - const intptr_t parent_len = smi_parent_len.Value(); - GET_NON_NULL_NATIVE_ARGUMENT(Smi, smi_total_len, arguments->NativeArgAt(3)); - const intptr_t total_len = smi_total_len.Value(); - const TypeArguments& result = - TypeArguments::Handle(zone, TypeArguments::New(total_len, Heap::kNew)); - AbstractType& type = AbstractType::Handle(zone); - for (intptr_t i = 0; i < parent_len; i++) { - type = parent_type_arguments.IsNull() ? Type::DynamicType() - : parent_type_arguments.TypeAt(i); - result.SetTypeAt(i, type); - } - for (intptr_t i = parent_len; i < total_len; i++) { - type = function_type_arguments.IsNull() - ? Type::DynamicType() - : function_type_arguments.TypeAt(i - parent_len); - result.SetTypeAt(i, type); - } - return result.Canonicalize(); + GET_NON_NULL_NATIVE_ARGUMENT(Smi, smi_len, arguments->NativeArgAt(3)); + return function_type_arguments.Prepend( + zone, parent_type_arguments, smi_parent_len.Value(), smi_len.Value()); } DEFINE_NATIVE_ENTRY(InvocationMirror_unpackTypeArguments, 1) { diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index ee02cbc152a..b0cea33c3db 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -5010,66 +5010,10 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfImplicitClosureFunction( body += flow_graph_builder_->CheckStackOverflowInPrologue(function.token_pos()); - // Forwarding the type parameters is complicated by our approach to - // implementing the partial tearoff instantiation. - // - // When a tearoff is partially applied to a set of type arguments, the type - // arguments are saved in the closure's "delayed_type_arguments" field. The - // partial type application operator is guaranteed to provide arguments for - // all of a generic tearoff's type parameters, so we will only have to forward - // type arguments from the caller or from the closure object. Therefore, if - // there are type arguments saved on the tearoff and type arguments are - // passed, we must throw NoSuchMethod. intptr_t type_args_len = 0; if (I->reify_generic_functions() && function.IsGeneric()) { + type_args_len = function.NumTypeParameters(); ASSERT(parsed_function()->function_type_arguments() != NULL); - type_args_len = target.NumTypeParameters(); - - Fragment copy_type_args; - - LocalVariable* closure = - parsed_function()->node_sequence()->scope()->VariableAt(0); - copy_type_args += LoadLocal(closure); - copy_type_args += LoadField(Closure::delayed_type_arguments_offset()); - - TargetEntryInstr *has_delayed_type_args, *no_delayed_type_args; - copy_type_args += Constant(Object::empty_type_arguments()); - copy_type_args += BranchIfEqual(&no_delayed_type_args, - &has_delayed_type_args, /*negate=*/false); - JoinEntryInstr* join = BuildJoinEntry(); - - // We found type arguments saved on the tearoff to be provided to the - // function. - - Fragment copy_delayed_type_args(has_delayed_type_args); - - copy_delayed_type_args += - LoadLocal(parsed_function()->function_type_arguments()); - - TargetEntryInstr *no_type_args, *passed_type_args; - copy_delayed_type_args += - BranchIfNull(&no_type_args, &passed_type_args, /*negate=*/false); - - Fragment use_instantiated_args(no_type_args); - use_instantiated_args += LoadLocal(closure); - use_instantiated_args += - LoadField(Closure::delayed_type_arguments_offset()); - - // Prepending of captured type arguments will happen in the target. - use_instantiated_args += StoreLocal( - TokenPosition::kNoSource, parsed_function()->function_type_arguments()); - use_instantiated_args += Drop(); - use_instantiated_args += Goto(join); - - Fragment goto_nsm(passed_type_args); - goto_nsm += Goto(flow_graph_builder_->BuildThrowNoSuchMethod()); - - // The tearoff was not partially applied, so we forward type arguments from - // the caller. - Fragment forward_caller_type_args(no_delayed_type_args); - forward_caller_type_args += Goto(join); - - body += Fragment(copy_type_args.entry, join); body += LoadLocal(parsed_function()->function_type_arguments()); body += PushArgument(); } diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index 1064875988d..aa1cb204b3a 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -111,6 +111,15 @@ Fragment& Fragment::operator<<=(Instruction* next) { return *this; } +void Fragment::Prepend(Instruction* start) { + if (entry == NULL) { + entry = current = start; + } else { + start->LinkTo(entry); + entry = start; + } +} + Fragment Fragment::closed() { ASSERT(entry != NULL); return Fragment(entry, NULL); @@ -1330,6 +1339,52 @@ Fragment BaseFlowGraphBuilder::TailCall(const Code& code) { return Fragment(new (Z) TailCallInstr(code, arg_desc)); } +Fragment BaseFlowGraphBuilder::TestTypeArgsLen(Fragment eq_branch, + Fragment neq_branch, + intptr_t num_type_args) { + Fragment test; + + TargetEntryInstr* eq_entry; + TargetEntryInstr* neq_entry; + + test += LoadArgDescriptor(); + test += LoadField(ArgumentsDescriptor::type_args_len_offset()); + test += IntConstant(num_type_args); + test += BranchIfEqual(&eq_entry, &neq_entry); + + eq_branch.Prepend(eq_entry); + neq_branch.Prepend(neq_entry); + + JoinEntryInstr* join = BuildJoinEntry(); + eq_branch += Goto(join); + neq_branch += Goto(join); + + return Fragment(test.entry, join); +} + +Fragment BaseFlowGraphBuilder::TestDelayedTypeArgs(LocalVariable* closure, + Fragment present, + Fragment absent) { + Fragment test; + + TargetEntryInstr* absent_entry; + TargetEntryInstr* present_entry; + + test += LoadLocal(closure); + test += LoadField(Closure::delayed_type_arguments_offset()); + test += Constant(Object::empty_type_arguments()); + test += BranchIfEqual(&absent_entry, &present_entry); + + present.Prepend(present_entry); + absent.Prepend(absent_entry); + + JoinEntryInstr* join = BuildJoinEntry(); + absent += Goto(join); + present += Goto(join); + + return Fragment(test.entry, join); +} + Fragment FlowGraphBuilder::RethrowException(TokenPosition position, int catch_try_index) { Fragment instructions; diff --git a/runtime/vm/compiler/frontend/kernel_to_il.h b/runtime/vm/compiler/frontend/kernel_to_il.h index 36557a114d5..817633fca35 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.h +++ b/runtime/vm/compiler/frontend/kernel_to_il.h @@ -7,6 +7,8 @@ #if !defined(DART_PRECOMPILED_RUNTIME) +#include + #include "vm/growable_array.h" #include "vm/hash_map.h" @@ -169,6 +171,12 @@ class Fragment { Fragment() : entry(NULL), current(NULL) {} + Fragment(std::initializer_list list) : entry(NULL), current(NULL) { + for (Fragment i : list) { + *this += i; + } + } + explicit Fragment(Instruction* instruction) : entry(instruction), current(instruction) {} @@ -178,6 +186,8 @@ class Fragment { bool is_open() { return entry == NULL || current != NULL; } bool is_closed() { return !is_open(); } + void Prepend(Instruction* start); + Fragment& operator+=(const Fragment& other); Fragment& operator<<=(Instruction* next); @@ -631,6 +641,13 @@ class BaseFlowGraphBuilder { return LoadLocal(parsed_function_->arg_desc_var()); } + Fragment TestTypeArgsLen(Fragment eq_branch, + Fragment neq_branch, + intptr_t num_type_args); + Fragment TestDelayedTypeArgs(LocalVariable* closure, + Fragment present, + Fragment absent); + JoinEntryInstr* BuildThrowNoSuchMethod(); protected: diff --git a/runtime/vm/compiler/frontend/prologue_builder.cc b/runtime/vm/compiler/frontend/prologue_builder.cc index 66a82c9923d..902b5b06ef8 100644 --- a/runtime/vm/compiler/frontend/prologue_builder.cc +++ b/runtime/vm/compiler/frontend/prologue_builder.cc @@ -58,7 +58,7 @@ BlockEntryInstr* PrologueBuilder::BuildPrologue(BlockEntryInstr* entry, if (!compiling_for_osr_) prologue += f; } if (expect_type_args) { - Fragment f = BuildTypeArgumentsHandling(strong); + Fragment f = BuildTypeArgumentsHandling(nsm); if (link) prologue += f; } @@ -391,36 +391,49 @@ Fragment PrologueBuilder::BuildClosureContextHandling() { return populate_context; } -Fragment PrologueBuilder::BuildTypeArgumentsHandling(bool strong) { - Fragment populate_args_desc; - +Fragment PrologueBuilder::BuildTypeArgumentsHandling(JoinEntryInstr* nsm) { LocalVariable* type_args_var = parsed_function_->RawTypeArgumentsVariable(); - TargetEntryInstr *passed, *not_passed; - populate_args_desc += LoadArgDescriptor(); - populate_args_desc += LoadField(ArgumentsDescriptor::type_args_len_offset()); - populate_args_desc += IntConstant(0); - populate_args_desc += BranchIfEqual(¬_passed, &passed); + Fragment handling; - JoinEntryInstr* join = BuildJoinEntry(); - - Fragment store_type_args(passed); + Fragment store_type_args; store_type_args += LoadArgDescriptor(); store_type_args += LoadField(ArgumentsDescriptor::count_offset()); store_type_args += LoadFpRelativeSlot(kWordSize * (1 + kParamEndSlotFromFp)); store_type_args += StoreLocal(TokenPosition::kNoSource, type_args_var); store_type_args += Drop(); - store_type_args += Goto(join); - Fragment store_null(not_passed); + Fragment store_null; store_null += NullConstant(); store_null += StoreLocal(TokenPosition::kNoSource, type_args_var); store_null += Drop(); - store_null += Goto(join); - populate_args_desc = Fragment(populate_args_desc.entry, join); + handling += TestTypeArgsLen(store_null, store_type_args, 0); - return populate_args_desc; + if (parsed_function_->function().IsClosureFunction()) { + LocalVariable* closure = + parsed_function_->node_sequence()->scope()->VariableAt(0); + + // Currently, delayed type arguments can only be introduced through type + // inference in the FE. So if they are present, we can assume they are + // correct in number and bound. + // clang-format off + Fragment use_delayed_type_args = { + LoadLocal(closure), + LoadField(Closure::delayed_type_arguments_offset()), + StoreLocal(TokenPosition::kNoSource, type_args_var), + Drop() + }; + + handling += TestDelayedTypeArgs( + closure, + /*present=*/TestTypeArgsLen( + use_delayed_type_args, Goto(nsm), 0), + /*absent=*/Fragment()); + // clang-format on + } + + return handling; } void PrologueBuilder::SortOptionalNamedParametersInto(LocalVariable** opt_param, diff --git a/runtime/vm/compiler/frontend/prologue_builder.h b/runtime/vm/compiler/frontend/prologue_builder.h index 5e896384164..2e1026fbf9a 100644 --- a/runtime/vm/compiler/frontend/prologue_builder.h +++ b/runtime/vm/compiler/frontend/prologue_builder.h @@ -60,7 +60,7 @@ class PrologueBuilder : public BaseFlowGraphBuilder { Fragment BuildClosureContextHandling(); - Fragment BuildTypeArgumentsHandling(bool strong); + Fragment BuildTypeArgumentsHandling(JoinEntryInstr* nsm); LocalVariable* ParameterVariable(intptr_t index) { return parsed_function_->RawParameterVariable(index); diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index ea2f2b25a5a..94ce9eebfe3 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -4856,6 +4856,27 @@ intptr_t TypeArguments::ComputeHash() const { return result; } +RawTypeArguments* TypeArguments::Prepend(Zone* zone, + const TypeArguments& other, + intptr_t other_length, + intptr_t total_length) const { + if (IsNull() && other.IsNull()) { + return TypeArguments::null(); + } + const TypeArguments& result = + TypeArguments::Handle(zone, TypeArguments::New(total_length, Heap::kNew)); + AbstractType& type = AbstractType::Handle(zone); + for (intptr_t i = 0; i < other_length; i++) { + type = other.IsNull() ? Type::DynamicType() : other.TypeAt(i); + result.SetTypeAt(i, type); + } + for (intptr_t i = other_length; i < total_length; i++) { + type = IsNull() ? Type::DynamicType() : TypeAt(i - other_length); + result.SetTypeAt(i, type); + } + return result.Canonicalize(); +} + RawString* TypeArguments::SubvectorName(intptr_t from_index, intptr_t len, NameVisibility name_visibility) const { @@ -22779,11 +22800,11 @@ RawFunction* Closure::GetInstantiatedSignature(Zone* zone) const { // We detect the case of a partial tearoff type application and substitute the // type arguments for the type parameters of the function. intptr_t num_free_params; - if (sig_fun.IsImplicitClosureFunction() && - delayed_type_args.raw() != Object::empty_type_arguments().raw()) { + if (delayed_type_args.raw() != Object::empty_type_arguments().raw()) { num_free_params = kCurrentAndEnclosingFree; - ASSERT(fn_type_args.IsNull()); // Implicit closure cannot have a parent. - fn_type_args = delayed_type_args.raw(); + fn_type_args = delayed_type_args.Prepend( + zone, fn_type_args, sig_fun.NumParentTypeParameters(), + sig_fun.NumTypeParameters() + sig_fun.NumParentTypeParameters()); } else { num_free_params = kAllFree; } diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 4bc1b9b2e9f..9787922c358 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -5755,6 +5755,11 @@ class TypeArguments : public Instance { return IsDynamicTypes(true, 0, len); } + RawTypeArguments* Prepend(Zone* zone, + const TypeArguments& other, + intptr_t other_length, + intptr_t total_length) const; + // Check if the subvector of length 'len' starting at 'from_index' of this // type argument vector consists solely of DynamicType, ObjectType, or // VoidType. diff --git a/tests/language_2/instantiate_tearoff_test.dart b/tests/language_2/instantiate_tearoff_test.dart index 1c50f7fbfa3..6a551f67dbf 100644 --- a/tests/language_2/instantiate_tearoff_test.dart +++ b/tests/language_2/instantiate_tearoff_test.dart @@ -8,10 +8,9 @@ T f(T x) => x; int intToInt(int x) => x; String stringToString(String x) => x; +String stringAndIntToString(String x, int y) => x; -main() { - int Function(int) intFunc = f; - dynamic intFuncDynamic = intFunc; +test(intFuncDynamic, stringFuncDynamic, dynamicFuncDynamic) { Expect.isTrue(intFuncDynamic is int Function(int)); Expect.isFalse(intFuncDynamic is String Function(String)); Expect.equals(intFuncDynamic(1), 1); @@ -22,8 +21,6 @@ main() { Expect.throwsNoSuchMethodError(() { intFuncDynamic('oops'); }); - String Function(String) stringFunc = f; - dynamic stringFuncDynamic = stringFunc; Expect.isTrue(stringFuncDynamic is String Function(String)); Expect.isFalse(stringFuncDynamic is int Function(int)); Expect.equals(stringFuncDynamic('hello'), 'hello'); @@ -35,9 +32,34 @@ main() { Expect.throwsNoSuchMethodError(() { stringFuncDynamic(1); }); - dynamic Function(dynamic) dynamicFunc = f; - dynamic dynamicFuncDynamic = dynamicFunc; Expect.throwsNoSuchMethodError(() { dynamicFuncDynamic(1); }); } + +main() { + int Function(int) if1 = f; + String Function(String) sf1 = f; + dynamic Function(dynamic) df1 = f; + test(if1, sf1, df1); + + T local(T x) => x; + + int Function(int) if2 = local; + String Function(String) sf2 = local; + dynamic Function(dynamic) df2 = local; + test(if2, sf2, df2); + + dynamic bar() { + String foo(X x, T t) { + return "$X, $T"; + } + + String Function(X, int) x = foo; + return x; + } + + dynamic fn = bar(); + Expect.equals("${fn.runtimeType}", "${stringAndIntToString.runtimeType}"); + Expect.equals(fn("a", 1), "String, int"); +} diff --git a/tests/language_2/language_2_kernel.status b/tests/language_2/language_2_kernel.status index c76e229af59..f53f4b5e12e 100644 --- a/tests/language_2/language_2_kernel.status +++ b/tests/language_2/language_2_kernel.status @@ -598,7 +598,6 @@ flatten_test/12: MissingRuntimeError for_in_side_effects_test/01: MissingCompileTimeError function_propagation_test: RuntimeError function_subtype_inline2_test: RuntimeError -generic_function_dcall_test: RuntimeError generic_instanceof2_test: RuntimeError generic_is_check_test: RuntimeError generic_no_such_method_dispatcher_simple_test: CompileTimeError # Issue 31533 @@ -931,7 +930,6 @@ flatten_test/12: MissingRuntimeError for_in_side_effects_test/01: MissingCompileTimeError function_propagation_test: RuntimeError function_subtype_inline2_test: RuntimeError -generic_function_dcall_test: RuntimeError generic_instanceof2_test: RuntimeError generic_is_check_test: RuntimeError generic_methods_recursive_bound_test/03: Crash, Pass diff --git a/tests/language_2/language_2_vm.status b/tests/language_2/language_2_vm.status index e71bf723c00..9d7067e287c 100644 --- a/tests/language_2/language_2_vm.status +++ b/tests/language_2/language_2_vm.status @@ -1135,6 +1135,7 @@ function_type_alias2_test: RuntimeError function_type_alias6_test/none: RuntimeError invalid_override_in_mixin_test/01: MissingCompileTimeError type_literal_prefix_call_test: RuntimeError +vm/regress_33040_instantiation_test: RuntimeError # The VM and does not implement the Dart 2.0 runtime checks yet unless # --checked is explicitly passed). diff --git a/tests/language_2/vm/regress_33040_instantiation_test.dart b/tests/language_2/vm/regress_33040_instantiation_test.dart new file mode 100644 index 00000000000..db536bbf278 --- /dev/null +++ b/tests/language_2/vm/regress_33040_instantiation_test.dart @@ -0,0 +1,35 @@ +// 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. +// +// Exact regression test for issue #33040. + +import 'dart:async'; + +class Optional {} + +typedef T ConvertFunction(); + +T blockingLatest() { + return null; +} + +abstract class ObservableModel {} + +abstract class AsyncValueMixin { + Future get asyncValue { + ConvertFunction f = blockingLatest; + (f as ConvertFunction); + return null; + } +} + +abstract class OptionalSettableObservableModel + extends ObservableModel> with AsyncValueMixin> {} + +class FooObservableModel extends OptionalSettableObservableModel {} + +void main() async { + var model = new FooObservableModel(); + final value = await model.asyncValue; +}