Skip to content

Commit

Permalink
[vm] Remove only use of Types of top level classes.
Browse files Browse the repository at this point in the history
The only time we create a Type in the VM for a top level class is as the
"receiver" for a no such method error where the method was either not
found in the top level class or the found method had incompatible
arguments.  Unlike other cases, the Dart implementation doesn't directly
use the InvocationMirror receiver for no such method errors at the top
level, but instead just passes it back to the VM.

Now, for the InvocationMirror receiver in the top level class case, we
either

* pass a null receiver if the method wasn't found, or

* pass the signature of the found method as a string, if the arguments
  were incompatible, since the only use of the top level class was for
  printing the signature.

TEST=vm/cc/DartAPI, service{_2}, vm/cc/Eval, lib{_2}/mirrors

Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: I1ac701b0be2aaebbc6163e644262c5f3ead656c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217220
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
  • Loading branch information
sstrickl authored and commit-bot@chromium.org committed Oct 20, 2021
1 parent c47cf76 commit 1b88870
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 31 deletions.
5 changes: 5 additions & 0 deletions runtime/lib/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,11 @@ DEFINE_NATIVE_ENTRY(NoSuchMethodError_existingMethodSignature, 0, 3) {
InvocationMirror::DecodeType(invocation_type.Value(), &level, &kind);

Function& function = Function::Handle(zone);
if (level == InvocationMirror::Level::kTopLevel) {
if (receiver.IsString()) return receiver.ptr();
ASSERT(receiver.IsNull());
return String::null();
}
if (receiver.IsType()) {
const auto& cls = Class::Handle(zone, Type::Cast(receiver).type_class());
const auto& error = Error::Handle(zone, cls.EnsureIsFinalized(thread));
Expand Down
17 changes: 12 additions & 5 deletions runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,8 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionBody(
} else if (has_body) {
body += BuildStatement();
} else if (dart_function.is_external()) {
body += ThrowNoSuchMethodError(dart_function);
body +=
ThrowNoSuchMethodError(dart_function, /*incompatible_arguments=*/false);
}

if (body.is_open()) {
Expand Down Expand Up @@ -1535,8 +1536,10 @@ Fragment StreamingFlowGraphBuilder::RethrowException(TokenPosition position,
}

Fragment StreamingFlowGraphBuilder::ThrowNoSuchMethodError(
const Function& target) {
return flow_graph_builder_->ThrowNoSuchMethodError(target);
const Function& target,
bool incompatible_arguments) {
return flow_graph_builder_->ThrowNoSuchMethodError(target,
incompatible_arguments);
}

Fragment StreamingFlowGraphBuilder::Constant(const Object& value) {
Expand Down Expand Up @@ -1571,7 +1574,9 @@ Fragment StreamingFlowGraphBuilder::StaticCall(TokenPosition position,
intptr_t argument_count,
ICData::RebindRule rebind_rule) {
if (!target.AreValidArgumentCounts(0, argument_count, 0, nullptr)) {
return flow_graph_builder_->ThrowNoSuchMethodError(target);
return flow_graph_builder_->ThrowNoSuchMethodError(
target,
/*incompatible_arguments=*/true);
}
return flow_graph_builder_->StaticCall(position, target, argument_count,
rebind_rule);
Expand All @@ -1588,7 +1593,9 @@ Fragment StreamingFlowGraphBuilder::StaticCall(
bool use_unchecked_entry) {
if (!target.AreValidArguments(type_args_count, argument_count, argument_names,
nullptr)) {
return flow_graph_builder_->ThrowNoSuchMethodError(target);
return flow_graph_builder_->ThrowNoSuchMethodError(
target,
/*incompatible_arguments=*/true);
}
return flow_graph_builder_->StaticCall(
position, target, argument_count, argument_names, rebind_rule,
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/compiler/frontend/kernel_binary_flowgraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
intptr_t yield_index = UntaggedPcDescriptors::kInvalidYieldIndex);
Fragment EvaluateAssertion();
Fragment RethrowException(TokenPosition position, int catch_try_index);
Fragment ThrowNoSuchMethodError(const Function& target);
Fragment ThrowNoSuchMethodError(const Function& target,
bool incompatible_arguments);
Fragment Constant(const Object& value);
Fragment IntConstant(int64_t value);
Fragment LoadStaticField(const Field& field, bool calls_initializer);
Expand Down
8 changes: 6 additions & 2 deletions runtime/vm/compiler/frontend/kernel_to_il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,8 @@ Fragment FlowGraphBuilder::ThrowTypeError() {
return instructions;
}

Fragment FlowGraphBuilder::ThrowNoSuchMethodError(const Function& target) {
Fragment FlowGraphBuilder::ThrowNoSuchMethodError(const Function& target,
bool incompatible_arguments) {
const Class& klass = Class::ZoneHandle(
Z, Library::LookupCoreClass(Symbols::NoSuchMethodError()));
ASSERT(!klass.IsNull());
Expand All @@ -687,7 +688,7 @@ Fragment FlowGraphBuilder::ThrowNoSuchMethodError(const Function& target) {
Fragment instructions;

const Class& owner = Class::Handle(Z, target.Owner());
AbstractType& receiver = AbstractType::ZoneHandle();
auto& receiver = Instance::ZoneHandle();
InvocationMirror::Kind kind = InvocationMirror::Kind::kMethod;
if (target.IsImplicitGetterFunction() || target.IsGetterFunction()) {
kind = InvocationMirror::kGetter;
Expand All @@ -696,6 +697,9 @@ Fragment FlowGraphBuilder::ThrowNoSuchMethodError(const Function& target) {
}
InvocationMirror::Level level;
if (owner.IsTopLevel()) {
if (incompatible_arguments) {
receiver = target.UserVisibleSignature();
}
level = InvocationMirror::Level::kTopLevel;
} else {
receiver = owner.RareType();
Expand Down
6 changes: 5 additions & 1 deletion runtime/vm/compiler/frontend/kernel_to_il.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,11 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
Fragment StringInterpolateSingle(TokenPosition position);
Fragment StringInterpolate(TokenPosition position);
Fragment ThrowTypeError();
Fragment ThrowNoSuchMethodError(const Function& target);

// [incompatible_arguments] should be true if the NSM is due to a mismatch
// between the provided arguments and the function signature.
Fragment ThrowNoSuchMethodError(const Function& target,
bool incompatible_arguments);
Fragment ThrowLateInitializationError(TokenPosition position,
const char* throw_method_name,
const String& name);
Expand Down
57 changes: 35 additions & 22 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4118,10 +4118,11 @@ void Class::SetTraceAllocation(bool trace_allocation) const {
}

// Conventions:
// * For throwing a NSM in a class klass we use its runtime type as receiver,
// i.e., klass.RareType().
// * For throwing a NSM in a library, we just pass the null instance as
// receiver.
// * For throwing a NSM in a library or top-level class (i.e., level is
// kTopLevel), if a method was found but was incompatible, we pass the
// signature of the found method as a string, otherwise the null instance.
// * Otherwise, for throwing a NSM in a class klass we use its runtime type as
// receiver, i.e., klass.RareType().
static ObjectPtr ThrowNoSuchMethod(const Instance& receiver,
const String& function_name,
const Array& arguments,
Expand All @@ -4131,6 +4132,8 @@ static ObjectPtr ThrowNoSuchMethod(const Instance& receiver,
const Smi& invocation_type =
Smi::Handle(Smi::New(InvocationMirror::EncodeType(level, kind)));

ASSERT(!receiver.IsNull() || level == InvocationMirror::Level::kTopLevel);
ASSERT(level != InvocationMirror::Level::kTopLevel || receiver.IsString());
const Array& args = Array::Handle(Array::New(7));
args.SetAt(0, receiver);
args.SetAt(1, function_name);
Expand Down Expand Up @@ -13361,10 +13364,10 @@ ObjectPtr Library::InvokeGetter(const String& getter_name,

if (getter.IsNull() || (respect_reflectable && !getter.is_reflectable())) {
if (throw_nsm_if_absent) {
return ThrowNoSuchMethod(
AbstractType::Handle(Class::Handle(toplevel_class()).RareType()),
getter_name, Object::null_array(), Object::null_array(),
InvocationMirror::kTopLevel, InvocationMirror::kGetter);
return ThrowNoSuchMethod(Object::null_string(), getter_name,
Object::null_array(), Object::null_array(),
InvocationMirror::kTopLevel,
InvocationMirror::kGetter);
}

// Fall through case: Indicate that we didn't find any function or field
Expand Down Expand Up @@ -13402,10 +13405,10 @@ ObjectPtr Library::InvokeSetter(const String& setter_name,
const Array& args = Array::Handle(Array::New(kNumArgs));
args.SetAt(0, value);

return ThrowNoSuchMethod(
AbstractType::Handle(Class::Handle(toplevel_class()).RareType()),
internal_setter_name, args, Object::null_array(),
InvocationMirror::kTopLevel, InvocationMirror::kSetter);
return ThrowNoSuchMethod(Object::null_string(), internal_setter_name,
args, Object::null_array(),
InvocationMirror::kTopLevel,
InvocationMirror::kSetter);
}
field.SetStaticValue(value);
return value.ptr();
Expand All @@ -13425,10 +13428,9 @@ ObjectPtr Library::InvokeSetter(const String& setter_name,
const Array& args = Array::Handle(Array::New(kNumArgs));
args.SetAt(0, value);
if (setter.IsNull() || (respect_reflectable && !setter.is_reflectable())) {
return ThrowNoSuchMethod(
AbstractType::Handle(Class::Handle(toplevel_class()).RareType()),
internal_setter_name, args, Object::null_array(),
InvocationMirror::kTopLevel, InvocationMirror::kSetter);
return ThrowNoSuchMethod(Object::null_string(), internal_setter_name, args,
Object::null_array(), InvocationMirror::kTopLevel,
InvocationMirror::kSetter);
}

setter_type = setter.ParameterTypeAt(0);
Expand Down Expand Up @@ -13491,13 +13493,15 @@ ObjectPtr Library::Invoke(const String& function_name,
}

if (function.IsNull() ||
!function.AreValidArguments(args_descriptor, nullptr) ||
(respect_reflectable && !function.is_reflectable())) {
return ThrowNoSuchMethod(Object::null_string(), function_name, args,
arg_names, InvocationMirror::kTopLevel,
InvocationMirror::kMethod);
}
if (!function.AreValidArguments(args_descriptor, nullptr)) {
return ThrowNoSuchMethod(
AbstractType::Handle(zone,
Class::Handle(zone, toplevel_class()).RareType()),
function_name, args, arg_names, InvocationMirror::kTopLevel,
InvocationMirror::kMethod);
String::Handle(function.UserVisibleSignature()), function_name, args,
arg_names, InvocationMirror::kTopLevel, InvocationMirror::kMethod);
}
// This is a static function, so we pass an empty instantiator tav.
ASSERT(function.is_static());
Expand Down Expand Up @@ -21257,7 +21261,16 @@ uword FunctionType::ComputeHash() const {

void Type::set_type_class(const Class& value) const {
ASSERT(!value.IsNull());
untag()->set_type_class_id(Smi::New(value.id()));
const intptr_t id = value.id();
// We should never need a Type object for a top-level class.
ASSERT(!ClassTable::IsTopLevelCid(id));
ASSERT(id == kIllegalCid || !IsInternalOnlyClassId(id));
// We must allow Types with kIllegalCid type class ids, because the class
// used for evaluating expressions inside a instance method call context
// from the debugger is not registered (and thus has kIllegalCid as an id).
// Thus, we check id != kIllegalCid only if IsType(), not if and only if.
ASSERT(id == kIllegalCid || IsType());
untag()->set_type_class_id(Smi::New(id));
}

void Type::set_arguments(const TypeArguments& value) const {
Expand Down

0 comments on commit 1b88870

Please sign in to comment.