[vm] Correct semantics of bounds-checks on partial instantiation.
Fixes #34267.
Change-Id: Ieb557e186aa457bac3b99b6817050bde8f8e142f
Reviewed-on: https://dart-review.googlesource.com/72161
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/lib/internal_patch.dart b/runtime/lib/internal_patch.dart
index 38e7250..b906776 100644
--- a/runtime/lib/internal_patch.dart
+++ b/runtime/lib/internal_patch.dart
@@ -102,6 +102,12 @@
_prependTypeArguments(functionTypeArguments, parentTypeArguments, parentLen,
totalLen) native "Internal_prependTypeArguments";
+// Check that a set of type arguments satisfy the type parameter bounds on a
+// closure.
+@pragma("vm:entry-point")
+_boundsCheckForPartialInstantiation(closure, type_args)
+ native "Internal_boundsCheckForPartialInstantiation";
+
// Called by IRRegExpMacroAssembler::GrowStack.
Int32List _growRegExpStack(Int32List stack) {
final newStack = new Int32List(stack.length * 2);
diff --git a/runtime/lib/object.cc b/runtime/lib/object.cc
index 3e01f83..b68fac0 100644
--- a/runtime/lib/object.cc
+++ b/runtime/lib/object.cc
@@ -473,6 +473,72 @@
zone, parent_type_arguments, smi_parent_len.Value(), smi_len.Value());
}
+// Check that a set of type arguments satisfy the type parameter bounds on a
+// closure.
+// Arg0: Closure object
+// Arg1: Type arguments to function
+DEFINE_NATIVE_ENTRY(Internal_boundsCheckForPartialInstantiation, 2) {
+ const Closure& closure =
+ Closure::CheckedHandle(zone, arguments->NativeArgAt(0));
+ const Function& target = Function::Handle(zone, closure.function());
+ const TypeArguments& bounds =
+ TypeArguments::Handle(zone, target.type_parameters());
+
+ // Either the bounds are all-dynamic or the function is not generic.
+ if (bounds.IsNull()) return Object::null();
+
+ const TypeArguments& type_args_to_check =
+ TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(1));
+
+ // This should be guaranteed by the front-end.
+ ASSERT(type_args_to_check.IsNull() ||
+ bounds.Length() == type_args_to_check.Length());
+
+ // The bounds on the closure may need instantiation.
+ const TypeArguments& instantiator_type_args =
+ TypeArguments::Handle(zone, closure.instantiator_type_arguments());
+ const TypeArguments& function_type_args =
+ TypeArguments::Handle(zone, closure.function_type_arguments());
+
+ AbstractType& supertype = AbstractType::Handle(zone);
+ AbstractType& subtype = AbstractType::Handle(zone);
+ TypeParameter& parameter = TypeParameter::Handle(zone);
+ for (intptr_t i = 0; i < bounds.Length(); ++i) {
+ parameter ^= bounds.TypeAt(i);
+ supertype = parameter.bound();
+ subtype = type_args_to_check.TypeAt(i);
+
+ ASSERT(!subtype.IsNull() && !subtype.IsMalformedOrMalbounded());
+ ASSERT(!supertype.IsNull() && !supertype.IsMalformedOrMalbounded());
+
+ // The supertype may not be instantiated.
+ Error& bound_error = Error::Handle(zone);
+ if (!AbstractType::InstantiateAndTestSubtype(
+ &subtype, &supertype, &bound_error, instantiator_type_args,
+ function_type_args)) {
+ // Throw a dynamic type error.
+ TokenPosition location;
+ {
+ DartFrameIterator iterator(Thread::Current(),
+ StackFrameIterator::kNoCrossThreadIteration);
+ StackFrame* caller_frame = iterator.NextFrame();
+ ASSERT(caller_frame != NULL);
+ location = caller_frame->GetTokenPos();
+ }
+ String& bound_error_message = String::Handle(zone);
+ if (!bound_error.IsNull()) {
+ bound_error_message = String::New(bound_error.ToErrorCString());
+ }
+ String& parameter_name = String::Handle(zone, parameter.Name());
+ Exceptions::CreateAndThrowTypeError(location, subtype, supertype,
+ parameter_name, bound_error_message);
+ UNREACHABLE();
+ }
+ }
+
+ return Object::null();
+}
+
DEFINE_NATIVE_ENTRY(InvocationMirror_unpackTypeArguments, 2) {
const TypeArguments& type_arguments =
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0));
diff --git a/runtime/vm/bootstrap_natives.h b/runtime/vm/bootstrap_natives.h
index 1fc543f..616abe6 100644
--- a/runtime/vm/bootstrap_natives.h
+++ b/runtime/vm/bootstrap_natives.h
@@ -313,6 +313,7 @@
V(Internal_inquireIs64Bit, 0) \
V(Internal_extractTypeArguments, 2) \
V(Internal_prependTypeArguments, 4) \
+ V(Internal_boundsCheckForPartialInstantiation, 2) \
V(InvocationMirror_unpackTypeArguments, 2) \
V(NoSuchMethodError_existingMethodSignature, 3) \
V(LinkedHashMap_getIndex, 1) \
diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
index f9bee81..ad4ad84 100644
--- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
@@ -4745,14 +4745,35 @@
Class::ZoneHandle(Z, I->object_store()->closure_class()), 0);
LocalVariable* new_closure = MakeTemporary();
- instructions += LoadLocal(new_closure);
-
intptr_t num_type_args = ReadListLength();
const TypeArguments& type_args = T.BuildTypeArguments(num_type_args);
instructions += TranslateInstantiatedTypeArguments(type_args);
+ LocalVariable* type_args_vec = MakeTemporary();
+
+ // 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();
+
+ instructions += LoadLocal(new_closure);
+ instructions += LoadLocal(type_args_vec);
instructions += StoreInstanceField(TokenPosition::kNoSource,
Closure::delayed_type_arguments_offset());
+ instructions += Drop(); // Drop type args.
+
// Copy over the target function.
instructions += LoadLocal(new_closure);
instructions += LoadLocal(original_closure);
@@ -4781,7 +4802,7 @@
instructions +=
StoreInstanceField(TokenPosition::kNoSource, Closure::context_offset());
- instructions += DropTempsPreserveTop(1); // drop old closure
+ instructions += DropTempsPreserveTop(1); // Drop old closure.
return instructions;
}
diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h
index 9109b3d..28c6b68 100644
--- a/runtime/vm/symbols.h
+++ b/runtime/vm/symbols.h
@@ -465,7 +465,8 @@
V(vm_entry_point, "vm:entry-point") \
V(Get, "get") \
V(Set, "set") \
- V(vm_trace_entrypoints, "vm:testing.unsafe.trace-entrypoints-fn")
+ V(vm_trace_entrypoints, "vm:testing.unsafe.trace-entrypoints-fn") \
+ V(BoundsCheckForPartialInstantiation, "_boundsCheckForPartialInstantiation")
// Contains a list of frequently used strings in a canonicalized form. This
// list is kept in the vm_isolate in order to share the copy across isolates
diff --git a/tests/language_2/language_2_dart2js.status b/tests/language_2/language_2_dart2js.status
index 302411e..a079eb5 100644
--- a/tests/language_2/language_2_dart2js.status
+++ b/tests/language_2/language_2_dart2js.status
@@ -67,6 +67,7 @@
mixin_type_parameter_inference_test/16: CompileTimeError
mixin_type_parameter_inference_test/none: CompileTimeError
number_identity_test: CompileTimeError, OK # Error if web int literal cannot be represented exactly, see http://dartbug.com/33351
+partial_instantiation_eager_bounds_check_test: RuntimeError # Issue #34295
partial_tearoff_instantiation_test/05: Pass # for the wrong reason.
partial_tearoff_instantiation_test/06: Pass # for the wrong reason.
partial_tearoff_instantiation_test/07: Pass # for the wrong reason.
diff --git a/tests/language_2/language_2_dartdevc.status b/tests/language_2/language_2_dartdevc.status
index a66eabb..0e8f40f 100644
--- a/tests/language_2/language_2_dartdevc.status
+++ b/tests/language_2/language_2_dartdevc.status
@@ -495,6 +495,7 @@
number_identity_test: CompileTimeError, OK # Error if web int literal cannot be represented exactly, see http://dartbug.com/33351
numbers_test: RuntimeError # Issue 29920; Expect.equals(expected: <false>, actual: <true>) fails.
parser_quirks_test: CompileTimeError
+partial_instantiation_eager_bounds_check_test: RuntimeError # Issue 34296
regress_16640_test: RuntimeError # Issue 29920; Uncaught Error: type arguments should not be null: E => {
regress_22443_test: RuntimeError # Uncaught Expect.isTrue(false) fails.
stack_overflow_stacktrace_test: RuntimeError # Issue 29920; RangeError: Maximum call stack size exceeded
diff --git a/tests/language_2/partial_instantiation_eager_bounds_check_test.dart b/tests/language_2/partial_instantiation_eager_bounds_check_test.dart
new file mode 100644
index 0000000..d20a83e
--- /dev/null
+++ b/tests/language_2/partial_instantiation_eager_bounds_check_test.dart
@@ -0,0 +1,21 @@
+// 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.
+//
+// This test checks that necessary type argument bounds checks are performed
+// eagerly during partial instantiation, rather than being delayed until the
+// partially instantiated closure is invoked.
+
+import "package:expect/expect.dart";
+
+class C<T> {
+ void foo<S extends T>(S x) {}
+}
+
+void main() {
+ C<Object> c = C<int>();
+ void Function(String) fn;
+ Expect.throwsTypeError(() {
+ fn = c.foo;
+ });
+}