Revert "[vm] Skip bounds checks on invocation of partially instantiated closures."
This reverts commit 5e34333c121796e5cc00045bf8ab77520fbabff3.
Reason for revert: Broke several bots.
Original change's description:
> [vm] Skip bounds checks on invocation of partially instantiated closures.
>
> Change-Id: I042607b6c255b20d8071dd51439a50bfd4c3bee7
> Reviewed-on: https://dart-review.googlesource.com/72460
> Commit-Queue: Samir Jindel <sjindel@google.com>
> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
TBR=vegorov@google.com,sjindel@google.com
Change-Id: Ie52f0da8e1a52d48dabff458bf391e3cb7f11d16
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/72402
Reviewed-by: Samir Jindel <sjindel@google.com>
Commit-Queue: Samir Jindel <sjindel@google.com>
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 7430fac..88cec81 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -45,6 +45,7 @@
unbox_numeric_fields,
!USING_DBC,
"Support unboxed double and float32x4 fields.");
+DECLARE_FLAG(bool, eliminate_type_checks);
class SubclassFinder {
public:
diff --git a/runtime/vm/compiler/frontend/flow_graph_builder.cc b/runtime/vm/compiler/frontend/flow_graph_builder.cc
index 847053c..4d93fdf 100644
--- a/runtime/vm/compiler/frontend/flow_graph_builder.cc
+++ b/runtime/vm/compiler/frontend/flow_graph_builder.cc
@@ -36,6 +36,10 @@
namespace dart {
+DEFINE_FLAG(bool,
+ eliminate_type_checks,
+ true,
+ "Eliminate type checks when allowed by static type analysis.");
DEFINE_FLAG(bool, print_ast, false, "Print abstract syntax tree.");
DEFINE_FLAG(bool, print_scopes, false, "Print scopes of local variables.");
DEFINE_FLAG(bool,
diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
index 51d3b26..ad4ad84 100644
--- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
@@ -965,7 +965,6 @@
I->strong() && I->reify_generic_functions();
TypeParameter& forwarding_param = TypeParameter::Handle(Z);
- Fragment check_bounds;
for (intptr_t i = 0; i < num_type_params; ++i) {
TypeParameterHelper helper(this);
helper.ReadUntilExcludingAndSetJustRead(TypeParameterHelper::kBound);
@@ -1010,20 +1009,7 @@
param ^= TypeArguments::Handle(dart_function.type_parameters()).TypeAt(i);
}
ASSERT(param.IsFinalized());
- check_bounds += CheckTypeArgumentBound(param, bound, name);
- }
-
- // Type arguments passed through partial instantiation are guaranteed to be
- // bounds-checked at the point of partial instantiation, so we don't need to
- // check them again at the call-site.
- if (dart_function.IsClosureFunction() && !check_bounds.is_empty() &&
- FLAG_eliminate_type_checks) {
- LocalVariable* closure =
- parsed_function()->node_sequence()->scope()->VariableAt(0);
- *implicit_checks += B->TestDelayedTypeArgs(closure, /*present=*/{},
- /*absent=*/check_bounds);
- } else {
- *implicit_checks += check_bounds;
+ *implicit_checks += CheckTypeArgumentBound(param, bound, name);
}
function_node_helper.SetJustRead(FunctionNodeHelper::kTypeParameters);
@@ -4751,7 +4737,6 @@
// Create a copy of the closure.
- Tag closure_expression_tag = PeekTag();
Fragment instructions = BuildExpression();
LocalVariable* original_closure = MakeTemporary();
@@ -4767,39 +4752,20 @@
// Check the bounds.
//
- // The Dart 2.0 spec only allows the partial instantiation operation to happen
- // against a "tearoff", which in the spec's language means a direct reference
- // to a local function, a direct reference to a toplevel function, or an
- // actual tearoff of a method against an object.
- //
- // In the first two cases the closure's expression will be represented by a
- // "VariableGet" or "StaticGet", and in these cases we don't need to perform
- // any checks, since the bounds are known statically by the front-end. This is
- // also true also in the case of a super-tearoff or tearoff by direct method
- // reference. Only in the latter case, where the closure's expression is
- // represented by a "PropertyGet", is the target resolved dynamically, and
- // checks are needed.
- ASSERT(closure_expression_tag == kPropertyGet ||
- closure_expression_tag == kDirectPropertyGet ||
- closure_expression_tag == kSuperPropertyGet ||
- closure_expression_tag == kVariableGet ||
- closure_expression_tag == kSpecializedVariableGet ||
- closure_expression_tag == kStaticGet);
- if (closure_expression_tag == kPropertyGet || !FLAG_eliminate_type_checks) {
- instructions += LoadLocal(original_closure);
- instructions += PushArgument();
- instructions += LoadLocal(type_args_vec);
- instructions += PushArgument();
- const Library& dart_internal =
- Library::Handle(Z, Library::InternalLibrary());
- const Function& bounds_check_function = Function::ZoneHandle(
- Z, dart_internal.LookupFunctionAllowPrivate(
- Symbols::BoundsCheckForPartialInstantiation()));
- ASSERT(!bounds_check_function.IsNull());
- instructions += StaticCall(TokenPosition::kNoSource, bounds_check_function,
- 2, ICData::kStatic);
- instructions += Drop();
- }
+ // TODO(sjindel): Only perform this check for instance tearoffs, not for
+ // tearoffs against local or top-level functions.
+ instructions += LoadLocal(original_closure);
+ instructions += PushArgument();
+ instructions += LoadLocal(type_args_vec);
+ instructions += PushArgument();
+ const Library& dart_internal = Library::Handle(Z, Library::InternalLibrary());
+ const Function& bounds_check_function = Function::ZoneHandle(
+ Z, dart_internal.LookupFunctionAllowPrivate(
+ Symbols::BoundsCheckForPartialInstantiation()));
+ ASSERT(!bounds_check_function.IsNull());
+ instructions += StaticCall(TokenPosition::kNoSource, bounds_check_function, 2,
+ ICData::kStatic);
+ instructions += Drop();
instructions += LoadLocal(new_closure);
instructions += LoadLocal(type_args_vec);
diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h
index 2f3600b..83c1162 100644
--- a/runtime/vm/flag_list.h
+++ b/runtime/vm/flag_list.h
@@ -214,8 +214,6 @@
P(enable_multiple_entrypoints, bool, true, \
"Enable multiple entrypoints per-function and related optimizations.") \
R(enable_testing_pragmas, false, bool, false, \
- "Enable magical pragmas for testing purposes. Use at your own risk!") \
- R(eliminate_type_checks, true, bool, true, \
- "Eliminate type checks when allowed by static type analysis.")
+ "Enable magical pragmas for testing purposes. Use at your own risk!")
#endif // RUNTIME_VM_FLAG_LIST_H_
diff --git a/tests/language_2/language_2.status b/tests/language_2/language_2.status
index 48b19d7..6a491e4 100644
--- a/tests/language_2/language_2.status
+++ b/tests/language_2/language_2.status
@@ -39,11 +39,6 @@
stacktrace_demangle_ctors_test: SkipByDesign # Names are not scrubbed.
type_checks_in_factory_method_test: SkipByDesign # Requires checked mode.
-[ $fasta ]
-partial_instantiation_static_bounds_check_test/01: MissingCompileTimeError # Issue 34327
-partial_instantiation_static_bounds_check_test/02: MissingCompileTimeError # Issue 34327
-partial_instantiation_static_bounds_check_test/03: MissingCompileTimeError # Issue 34327
-
[ $compiler != app_jitk && $compiler != dartk && $compiler != dartkb && $compiler != dartkp && $mode == debug && $runtime == vm ]
built_in_identifier_type_annotation_test/set: Crash # Not supported by legacy VM front-end.
diff --git a/tests/language_2/partial_instantiation_static_bounds_check_test.dart b/tests/language_2/partial_instantiation_static_bounds_check_test.dart
deleted file mode 100644
index 599574d..0000000
--- a/tests/language_2/partial_instantiation_static_bounds_check_test.dart
+++ /dev/null
@@ -1,24 +0,0 @@
-topFn<T extends num>(T x) {
- print(T);
-}
-
-class C<T> {
- instanceFn<S extends T>(S x) {
- print(S);
- }
-}
-
-class D<T> extends C<T> {
- void foo() {
- void Function(int) k = instanceFn; //# 03: compile-time error
- }
-}
-
-void main() {
- localFn<T extends num>(T x) {
- print(T);
- }
-
- void Function(String) k0 = localFn; //# 01: compile-time error
- void Function(String) k1 = topFn; //# 02: compile-time error
-}
