[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>
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 88cec81..7430fac 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -45,7 +45,6 @@
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 4d93fdf..847053c 100644
--- a/runtime/vm/compiler/frontend/flow_graph_builder.cc
+++ b/runtime/vm/compiler/frontend/flow_graph_builder.cc
@@ -36,10 +36,6 @@
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 ad4ad84..51d3b26 100644
--- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
@@ -965,6 +965,7 @@
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);
@@ -1009,7 +1010,20 @@
param ^= TypeArguments::Handle(dart_function.type_parameters()).TypeAt(i);
}
ASSERT(param.IsFinalized());
- *implicit_checks += CheckTypeArgumentBound(param, bound, name);
+ 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;
}
function_node_helper.SetJustRead(FunctionNodeHelper::kTypeParameters);
@@ -4737,6 +4751,7 @@
// Create a copy of the closure.
+ Tag closure_expression_tag = PeekTag();
Fragment instructions = BuildExpression();
LocalVariable* original_closure = MakeTemporary();
@@ -4752,20 +4767,39 @@
// Check the bounds.
//
- // 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();
+ // 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();
+ }
instructions += LoadLocal(new_closure);
instructions += LoadLocal(type_args_vec);
diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h
index 83c1162..2f3600b 100644
--- a/runtime/vm/flag_list.h
+++ b/runtime/vm/flag_list.h
@@ -214,6 +214,8 @@
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!")
+ "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.")
#endif // RUNTIME_VM_FLAG_LIST_H_
diff --git a/tests/language_2/language_2.status b/tests/language_2/language_2.status
index 6a491e4..48b19d7 100644
--- a/tests/language_2/language_2.status
+++ b/tests/language_2/language_2.status
@@ -39,6 +39,11 @@
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
new file mode 100644
index 0000000..599574d
--- /dev/null
+++ b/tests/language_2/partial_instantiation_static_bounds_check_test.dart
@@ -0,0 +1,24 @@
+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
+}
