languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-16 03:17:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

[vm/compiler] Unify name checking in dynamic closure call dispatchers.

Browse source 
Instead of possibly looping over the closure function parameter names
array multiple times, just do it once. Check each entry against the
provided argument names. If none match, check the required bit (if in
null-safe mode). Also keep a count of matched names to check that all
provided names matched after the iteration.

Now that optional name checking is part of the fragment built by
BuildClosureCallArgumentsValidCheck, that method builds checks for
exactly the same things as its namesake in Function.

Also refactor the variables used by the checker into read/write ones
that need to be allocated in the parsed function and read-only ones
that can just be temporaries since no Phi nodes are needed.

Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm_x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try
Bug: https://github.com/dart-lang/sdk/issues/40813
Change-Id: I3cb421dd538629d7f5499f3bbf0653d34b850dce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/160725
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
This commit is contained in:
Tess Strickland 2020-09-10 07:12:14 +00:00 committed by commit-bot@chromium.org
parent b0acaea1e5
commit 3d77741666
10 changed files with 374 additions and 355 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
runtime/vm/compiler/frontend/base_flow_graph_builder.cc
View file

@ -682,10 +682,15 @@ Fragment BaseFlowGraphBuilder::StoreLocalRaw(TokenPosition position,
return instructions;
}
LocalVariable* BaseFlowGraphBuilder::MakeTemporary() {
char name[64];
LocalVariable* BaseFlowGraphBuilder::MakeTemporary(const char* suffix) {
static constexpr intptr_t kTemporaryNameLength = 64;
char name[kTemporaryNameLength];
intptr_t index = stack_->definition()->temp_index();
Utils::SNPrint(name, 64, ":t%" Pd, index);
if (suffix != nullptr) {
Utils::SNPrint(name, kTemporaryNameLength, ":t_%s", suffix);
} else {
Utils::SNPrint(name, kTemporaryNameLength, ":t%" Pd, index);
}
const String& symbol_name =
String::ZoneHandle(Z, Symbols::New(thread_, name));
LocalVariable* variable =
@ -707,6 +712,16 @@ LocalVariable* BaseFlowGraphBuilder::MakeTemporary() {
return variable;
}
Fragment BaseFlowGraphBuilder::DropTemporary(LocalVariable** temp) {
ASSERT(temp != nullptr && *temp != nullptr && (*temp)->HasIndex());
// Check that the temporary matches the current stack definition.
ASSERT_EQUAL(
stack_->definition()->temp_index(),
-(*temp)->index().value() - parsed_function_->num_stack_locals());
*temp = nullptr; // Clear to avoid inadvertent usage after dropping.
return Drop();
}
void BaseFlowGraphBuilder::SetTempIndex(Definition* definition) {
definition->set_temp_index(
stack_ == NULL ? 0 : stack_->definition()->temp_index() + 1);

7
runtime/vm/compiler/frontend/base_flow_graph_builder.h
View file

@ -183,6 +183,9 @@ class BaseFlowGraphBuilder {
void SetTempIndex(Definition* definition);
Fragment LoadLocal(LocalVariable* variable);
Fragment StoreLocal(LocalVariable* variable) {
return StoreLocal(TokenPosition::kNoSource, variable);
}
Fragment StoreLocal(TokenPosition position, LocalVariable* variable);
Fragment StoreLocalRaw(TokenPosition position, LocalVariable* variable);
Fragment LoadContextAt(int depth);
@ -242,8 +245,8 @@ class BaseFlowGraphBuilder {
// goto B3
// B3:
// LoadLocal(t)
//
LocalVariable* MakeTemporary();
LocalVariable* MakeTemporary(const char* suffix = nullptr);
Fragment DropTemporary(LocalVariable** temp);
InputsArray* GetArguments(int count);

575
runtime/vm/compiler/frontend/kernel_to_il.cc
View file

@ -1929,84 +1929,124 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfNoSuchMethodDispatcher(
prologue_info);
}
Fragment FlowGraphBuilder::BuildDynamicCallVarsInit(LocalVariable* closure) {
auto const vars = parsed_function_->dynamic_closure_call_vars();
ASSERT(vars != nullptr);
ASSERT(has_saved_args_desc_array());
const ArgumentsDescriptor descriptor(saved_args_desc_array());
auto const rep = Slot::Function_packed_fields().representation();
// Information used by the various dynamic closure call fragment builders.
struct FlowGraphBuilder::ClosureCallInfo {
ClosureCallInfo(LocalVariable* closure,
JoinEntryInstr* throw_no_such_method,
const Array& arguments_descriptor_array,
ParsedFunction::DynamicClosureCallVars* const vars)
: closure(ASSERT_NOTNULL(closure)),
throw_no_such_method(ASSERT_NOTNULL(throw_no_such_method)),
descriptor(arguments_descriptor_array),
vars(ASSERT_NOTNULL(vars)) {}
// We extract all the packed fields here so code generation that puts unboxed
// integers on the expression stack even in unoptimized code is in one place.
LocalVariable* const closure;
JoinEntryInstr* const throw_no_such_method;
const ArgumentsDescriptor descriptor;
ParsedFunction::DynamicClosureCallVars* const vars;
Fragment init;
init += LoadLocal(closure);
init += LoadNativeField(Slot::Closure_function());
init += LoadNativeField(Slot::Function_packed_fields());
init +=
BuildExtractPackedFieldIntoSmi<Function::PackedNumFixedParameters>(rep);
init += StoreLocal(TokenPosition::kNoSource, vars->num_fixed_params);
// Not dropping as we'll use the value to get the max number of parameters.
// Set up by BuildDynamicCallChecks() when needed. These values are
// read-only, so they don't need real local variables and are created
// using MakeTemporary().
LocalVariable* function = nullptr;
LocalVariable* num_fixed_params = nullptr;
LocalVariable* num_opt_params = nullptr;
LocalVariable* num_max_params = nullptr;
LocalVariable* has_named_params = nullptr;
LocalVariable* parameter_names = nullptr;
LocalVariable* type_parameters = nullptr;
};
init += LoadLocal(closure);
init += LoadNativeField(Slot::Closure_function());
init += LoadNativeField(Slot::Function_packed_fields());
init += BuildExtractPackedFieldIntoSmi<Function::PackedNumOptionalParameters>(
rep);
init += StoreLocal(TokenPosition::kNoSource, vars->num_opt_params);
init += SmiBinaryOp(Token::kADD);
init += StoreLocal(TokenPosition::kNoSource, vars->num_max_params);
init += Drop();
Fragment FlowGraphBuilder::TestClosureFunctionNamedParameterRequired(
const ClosureCallInfo& info,
Fragment set,
Fragment not_set) {
// Required named arguments only exist if null_safety is enabled.
if (!I->null_safety()) return not_set;
// Currently, we only need this initialized to either check provided optional
// names, if any, or to check for missing required parameters if null safe.
if (Isolate::Current()->null_safety() || descriptor.NamedCount() > 0) {
init += LoadLocal(closure);
init += LoadNativeField(Slot::Closure_function());
init += LoadNativeField(Slot::Function_parameter_names());
init += StoreLocal(TokenPosition::kNoSource, vars->parameter_names);
init += Drop();
}
Fragment check_required;
// First, we convert the index to be in terms of the number of optional
// parameters, not total parameters (to calculate the flag index and shift).
check_required += LoadLocal(info.vars->current_param_index);
check_required += LoadLocal(info.num_fixed_params);
check_required += SmiBinaryOp(Token::kSUB, /*is_truncating=*/true);
LocalVariable* opt_index = MakeTemporary("opt_index"); // Read-only.
init += LoadLocal(closure);
init += LoadNativeField(Slot::Closure_function());
init += LoadNativeField(Slot::Function_packed_fields());
init += BuildExtractPackedFieldIntoSmi<
Function::PackedHasNamedOptionalParameters>(rep);
init += IntConstant(0);
TargetEntryInstr *is_true, *is_false;
init += BranchIfEqual(&is_false, &is_true);
// Next, we calculate the index to dereference in the parameter names array.
check_required += LoadLocal(opt_index);
check_required +=
IntConstant(compiler::target::kNumParameterFlagsPerElementLog2);
check_required += SmiBinaryOp(Token::kSHR);
check_required += LoadLocal(info.num_max_params);
check_required += SmiBinaryOp(Token::kADD);
LocalVariable* flags_index = MakeTemporary("flags_index"); // Read-only.
JoinEntryInstr* join = BuildJoinEntry();
init.current = join;
// Two read-only stack values (opt_index, flag_index) that must be dropped
// after we rejoin at after_check.
JoinEntryInstr* after_check = BuildJoinEntry();
Fragment true_branch(is_true);
true_branch += Constant(Object::bool_true());
true_branch += StoreLocal(TokenPosition::kNoSource, vars->has_named_params);
true_branch += Drop();
true_branch += Goto(join);
// Now we check to see if the flags index is within the bounds of the
// parameters names array. If not, it cannot be required.
check_required += LoadLocal(flags_index);
check_required += LoadLocal(info.parameter_names);
check_required += LoadNativeField(Slot::Array_length());
check_required += SmiRelationalOp(Token::kLT);
TargetEntryInstr *valid_index, *invalid_index;
check_required += BranchIfTrue(&valid_index, &invalid_index);
Fragment false_branch(is_false);
false_branch += Constant(Object::bool_false());
false_branch += StoreLocal(TokenPosition::kNoSource, vars->has_named_params);
false_branch += Drop();
false_branch += Goto(join);
JoinEntryInstr* join_not_set = BuildJoinEntry();
return init;
Fragment(invalid_index) + Goto(join_not_set);
// Otherwise, we need to retrieve the value. We're guaranteed the Smis in
// the flag slots are non-null, so after loading we can immediate check
// the required flag bit for the given named parameter.
check_required.current = valid_index;
check_required += LoadLocal(info.parameter_names);
check_required += LoadLocal(flags_index);
check_required += LoadIndexed(compiler::target::kWordSize);
check_required += LoadLocal(opt_index);
check_required +=
IntConstant(compiler::target::kNumParameterFlagsPerElement - 1);
check_required += SmiBinaryOp(Token::kBIT_AND);
// If the below changes, we'll need to multiply by the number of parameter
// flags before shifting.
static_assert(compiler::target::kNumParameterFlags == 1,
"IL builder assumes only one flag bit per parameter");
check_required += SmiBinaryOp(Token::kSHR);
check_required +=
IntConstant(1 << compiler::target::kRequiredNamedParameterFlag);
check_required += SmiBinaryOp(Token::kBIT_AND);
check_required += IntConstant(0);
TargetEntryInstr *is_not_set, *is_set;
check_required += BranchIfEqual(&is_not_set, &is_set);
Fragment(is_not_set) + Goto(join_not_set);
set.Prepend(is_set);
set += Goto(after_check);
not_set.Prepend(join_not_set);
not_set += Goto(after_check);
// After rejoining, drop the introduced temporaries.
check_required.current = after_check;
check_required += DropTemporary(&flags_index);
check_required += DropTemporary(&opt_index);
return check_required;
}
Fragment FlowGraphBuilder::BuildClosureCallHasRequiredNamedArgumentsCheck(
LocalVariable* closure,
JoinEntryInstr* nsm) {
auto const vars = parsed_function_->dynamic_closure_call_vars();
ASSERT(vars != nullptr);
ASSERT(has_saved_args_desc_array());
const ArgumentsDescriptor descriptor(saved_args_desc_array());
// Required named arguments only exist if null_safety is enabled.
if (!Isolate::Current()->null_safety()) return Fragment();
if (descriptor.NamedCount() == 0) {
Fragment FlowGraphBuilder::BuildClosureCallNamedArgumentsCheck(
const ClosureCallInfo& info) {
// When no named arguments are provided, we just need to check for possible
// required named arguments.
if (info.descriptor.NamedCount() == 0) {
// No work to do if there are no possible required named parameters.
if (!I->null_safety()) {
return Fragment();
}
// If the below changes, we can no longer assume that flag slots existing
// means there are required parameters.
static_assert(compiler::target::kNumParameterFlags == 1,
"IL builder assumes only one flag bit per parameter");
// No named args were provided, so check for any required named params.
@ -2014,193 +2054,177 @@ Fragment FlowGraphBuilder::BuildClosureCallHasRequiredNamedArgumentsCheck(
// for named parameters. If this changes, we'll need to check each flag
// entry appropriately for any set required bits.
Fragment has_any;
has_any += LoadLocal(vars->num_max_params);
has_any += LoadLocal(vars->parameter_names);
has_any += LoadLocal(info.num_max_params);
has_any += LoadLocal(info.parameter_names);
has_any += LoadNativeField(Slot::Array_length());
TargetEntryInstr *no_required, *has_required;
has_any += BranchIfEqual(&no_required, &has_required);
Fragment(has_required) + Goto(nsm);
Fragment(has_required) + Goto(info.throw_no_such_method);
return Fragment(has_any.entry, no_required);
}
// Loop over the indexes of the named parameters of the function, checking
// whether the named parameter at that index is required. If it is, then
// check whether it matches any of the names in the ArgumentsDescriptor.
JoinEntryInstr* loop = BuildJoinEntry();
// Otherwise, we need to loop through the parameter names to check the names
// of named arguments for validity (and possibly missing required ones).
Fragment check_names;
check_names += LoadLocal(info.vars->current_param_index);
LocalVariable* old_index = MakeTemporary("old_index"); // Read-only.
check_names += LoadLocal(info.vars->current_num_processed);
LocalVariable* old_processed = MakeTemporary("old_processed"); // Read-only.
// We iterate from [0, num_named), not [num_fixed, num_named) because the
// flag mask and index is based off the named index, not the param index.
Fragment check_required;
check_required += IntConstant(0);
check_required +=
StoreLocal(TokenPosition::kNoSource, vars->current_param_index);
check_required += Drop();
check_required += Goto(loop);
// Two local stack values (old_index, old_processed) to drop after rejoining
// at done.
JoinEntryInstr* loop = BuildJoinEntry();
JoinEntryInstr* done = BuildJoinEntry();
check_names += IntConstant(0);
check_names += StoreLocal(info.vars->current_num_processed);
check_names += Drop();
check_names += LoadLocal(info.num_fixed_params);
check_names += StoreLocal(info.vars->current_param_index);
check_names += Drop();
check_names += Goto(loop);
Fragment loop_check(loop);
loop_check += LoadLocal(vars->current_param_index);
loop_check += LoadLocal(vars->num_opt_params);
loop_check += LoadLocal(info.vars->current_param_index);
loop_check += LoadLocal(info.num_max_params);
loop_check += SmiRelationalOp(Token::kLT);
TargetEntryInstr *no_more, *more;
loop_check += BranchIfTrue(&more, &no_more);
JoinEntryInstr* done = BuildJoinEntry();
Fragment(no_more) + Goto(done);
Fragment loop_body(more);
// First, we calculate the index to dereference into the parameter names
// array and store it in :expr_temp.
loop_body += LoadLocal(vars->num_max_params);
loop_body += LoadLocal(vars->current_param_index);
loop_body += IntConstant(compiler::target::kNumParameterFlagsPerElementLog2);
loop_body += SmiBinaryOp(Token::kSHR);
loop_body += SmiBinaryOp(Token::kADD);
LocalVariable* temp = parsed_function_->expression_temp_var();
loop_body += StoreLocal(TokenPosition::kNoSource, temp);
// Now we check to see if it is within the bounds of the parameters names
// array. If not, we're done, as this and later indices cannot be required.
loop_body += LoadLocal(vars->parameter_names);
loop_body += LoadNativeField(Slot::Array_length());
loop_body += SmiRelationalOp(Token::kLT);
TargetEntryInstr *valid_index, *invalid_index;
loop_body += BranchIfTrue(&valid_index, &invalid_index);
Fragment(invalid_index) + Goto(done);
// Otherwise, we need to retrieve the value and check the appropriate bit.
loop_body.current = valid_index;
loop_body += LoadLocal(vars->parameter_names);
loop_body += LoadLocal(temp); // Index into parameter names array.
// First load the name we need to check against.
loop_body += LoadLocal(info.parameter_names);
loop_body += LoadLocal(info.vars->current_param_index);
loop_body += LoadIndexed(compiler::target::kWordSize);
loop_body += LoadLocal(vars->current_param_index);
loop_body += IntConstant(compiler::target::kNumParameterFlagsPerElement - 1);
loop_body += SmiBinaryOp(Token::kBIT_AND);
if (compiler::target::kNumParameterFlags > 1) {
loop_body += IntConstant(compiler::target::kNumParameterFlags);
loop_body += SmiBinaryOp(Token::kMUL, /*is_truncating=*/false);
LocalVariable* param_name = MakeTemporary("param_name"); // Read only.
// One additional local value on the stack within the loop body (param_name)
// that should be dropped after rejoining at loop_incr.
JoinEntryInstr* loop_incr = BuildJoinEntry();
// Now iterate over the ArgumentsDescriptor names and check for a match.
for (intptr_t i = 0; i < info.descriptor.NamedCount(); i++) {
const auto& name = String::ZoneHandle(Z, info.descriptor.NameAt(i));
loop_body += Constant(name);
loop_body += LoadLocal(param_name);
TargetEntryInstr *match, *mismatch;
loop_body += BranchIfEqual(&match, &mismatch);
loop_body.current = mismatch;
// We have a match, so go to the next name after incrementing the number
// of matched arguments. (No need to check for the required bit, as this
// parameter was provided.)
Fragment matched(match);
matched += LoadLocal(info.vars->current_num_processed);
matched += IntConstant(1);
matched += SmiBinaryOp(Token::kADD, /*is_truncating=*/true);
matched += StoreLocal(info.vars->current_num_processed);
matched += Drop();
matched += Goto(loop_incr);
}
loop_body += SmiBinaryOp(Token::kSHR);
loop_body += IntConstant(1 << compiler::target::kRequiredNamedParameterFlag);
loop_body += SmiBinaryOp(Token::kBIT_AND);
loop_body += IntConstant(0);
TargetEntryInstr *not_set, *set;
loop_body += BranchIfEqual(&not_set, &set);
// Make a join entry for the increment at the end of the loop, so we can jump
// to it if we match one of the names in the ArgumentsDescriptor.
JoinEntryInstr* incr_index = BuildJoinEntry();
Fragment(not_set) + Goto(incr_index);
// None of the names in the arguments descriptor matched, so check if this
// is a required parameter.
loop_body += TestClosureFunctionNamedParameterRequired(
info,
/*set=*/Goto(info.throw_no_such_method),
/*not_set=*/{});
Fragment check_names(set);
if (descriptor.NamedCount() > 0) {
// First load the name we need to check against into :expr_temp.
check_names += LoadLocal(vars->parameter_names);
check_names += LoadLocal(vars->current_param_index);
check_names += LoadLocal(vars->num_fixed_params);
check_names += SmiBinaryOp(Token::kADD, /*is_truncating=*/true);
check_names += LoadIndexed(compiler::target::kWordSize);
check_names += StoreLocal(TokenPosition::kNoSource, temp);
check_names += Drop();
// Now iterate over the names in the ArgumentsDescriptor and add a check
// against each that goes to t he next loop iteration if the name is found.
for (intptr_t i = 0; i < descriptor.NamedCount(); i++) {
const auto& name = String::ZoneHandle(Z, descriptor.NameAt(i));
check_names += LoadLocal(temp);
check_names += Constant(name);
TargetEntryInstr *str_equal, *str_neq;
check_names += BranchIfEqual(&str_equal, &str_neq);
check_names.current = str_neq;
loop_body += Goto(loop_incr);
Fragment(str_equal) + Goto(incr_index);
}
}
// None of the names in the arguments descriptor matched, so throw NSM.
check_names += Goto(nsm);
Fragment incr_index(loop_incr);
incr_index += DropTemporary(&param_name);
incr_index += LoadLocal(info.vars->current_param_index);
incr_index += IntConstant(1);
incr_index += SmiBinaryOp(Token::kADD, /*is_truncating=*/true);
incr_index += StoreLocal(info.vars->current_param_index);
incr_index += Drop();
incr_index += Goto(loop);
// Increment the counter if the current parameter wasn't required or was
// required but provided.
loop_body.current = incr_index;
loop_body += LoadLocal(vars->current_param_index);
loop_body += IntConstant(1);
loop_body += SmiBinaryOp(Token::kADD, /*is_truncating=*/true);
loop_body += StoreLocal(TokenPosition::kNoSource, vars->current_param_index);
loop_body += Drop();
loop_body += Goto(loop);
Fragment check_processed(done);
check_processed += LoadLocal(info.vars->current_num_processed);
check_processed += IntConstant(info.descriptor.NamedCount());
TargetEntryInstr *all_processed, *bad_name;
check_processed += BranchIfEqual(&all_processed, &bad_name);
check_required.current = done;
return check_required;
// Didn't find a matching parameter name for at least one argument name.
Fragment(bad_name) + Goto(info.throw_no_such_method);
// Drop the temporaries at the end of the fragment.
check_names.current = all_processed;
check_names += LoadLocal(old_processed);
check_names += StoreLocal(info.vars->current_num_processed);
check_names += Drop();
check_names += DropTemporary(&old_processed);
check_names += LoadLocal(old_index);
check_names += StoreLocal(info.vars->current_param_index);
check_names += Drop();
check_names += DropTemporary(&old_index);
return check_names;
}
Fragment FlowGraphBuilder::BuildClosureCallArgumentsValidCheck(
LocalVariable* closure,
JoinEntryInstr* nsm) {
auto const vars = parsed_function_->dynamic_closure_call_vars();
ASSERT(vars != nullptr);
ASSERT(has_saved_args_desc_array());
const ArgumentsDescriptor descriptor(saved_args_desc_array());
LocalVariable* temp = parsed_function_->expression_temp_var();
const ClosureCallInfo& info) {
Fragment check_entry;
// We only need to check the length of any explicitly provided type arguments.
if (descriptor.TypeArgsLen() > 0) {
if (info.descriptor.TypeArgsLen() > 0) {
Fragment check_type_args_length;
check_type_args_length += LoadLocal(closure);
check_type_args_length += LoadNativeField(Slot::Closure_function());
check_type_args_length += LoadNativeField(Slot::Function_type_parameters());
check_type_args_length += StoreLocal(TokenPosition::kNoSource, temp);
check_type_args_length += LoadLocal(info.type_parameters);
TargetEntryInstr *null, *not_null;
check_type_args_length += BranchIfNull(&null, &not_null);
check_type_args_length.current = not_null; // Continue in non-error case.
// The function is not generic.
Fragment(null) + Goto(nsm);
check_type_args_length += LoadLocal(temp);
check_type_args_length += LoadLocal(info.type_parameters);
check_type_args_length += LoadNativeField(Slot::TypeArguments_length());
check_type_args_length += IntConstant(descriptor.TypeArgsLen());
check_type_args_length += IntConstant(info.descriptor.TypeArgsLen());
TargetEntryInstr *equal, *not_equal;
check_type_args_length += BranchIfEqual(&equal, &not_equal);
check_type_args_length.current = equal; // Continue in non-error case.
Fragment(not_equal) + Goto(nsm);
// The function is not generic.
Fragment(null) + Goto(info.throw_no_such_method);
// An incorrect number of type arguments were passed.
Fragment(not_equal) + Goto(info.throw_no_such_method);
// Type arguments should not be provided if there are delayed type
// arguments, as then the closure itself is not generic.
check_entry += TestDelayedTypeArgs(closure, /*present=*/Goto(nsm),
/*absent=*/check_type_args_length);
check_entry += TestDelayedTypeArgs(
info.closure, /*present=*/Goto(info.throw_no_such_method),
/*absent=*/check_type_args_length);
}
check_entry += LoadLocal(vars->has_named_params);
check_entry += LoadLocal(info.has_named_params);
TargetEntryInstr *has_named, *has_positional;
check_entry += BranchIfTrue(&has_named, &has_positional);
JoinEntryInstr* join_after_optional = BuildJoinEntry();
check_entry.current = join_after_optional;
if (descriptor.NamedCount() > 0) {
if (info.descriptor.NamedCount() > 0) {
// No reason to continue checking, as this function doesn't take named args.
Fragment(has_positional) + Goto(nsm);
Fragment(has_positional) + Goto(info.throw_no_such_method);
} else {
Fragment check_pos(has_positional);
check_pos += LoadLocal(vars->num_fixed_params);
check_pos += IntConstant(descriptor.PositionalCount());
check_pos += LoadLocal(info.num_fixed_params);
check_pos += IntConstant(info.descriptor.PositionalCount());
check_pos += SmiRelationalOp(Token::kLTE);
TargetEntryInstr *enough, *too_few;
check_pos += BranchIfTrue(&enough, &too_few);
check_pos.current = enough;
Fragment(too_few) + Goto(nsm);
Fragment(too_few) + Goto(info.throw_no_such_method);
check_pos += IntConstant(descriptor.PositionalCount());
check_pos += LoadLocal(vars->num_max_params);
check_pos += IntConstant(info.descriptor.PositionalCount());
check_pos += LoadLocal(info.num_max_params);
check_pos += SmiRelationalOp(Token::kLTE);
TargetEntryInstr *valid, *too_many;
check_pos += BranchIfTrue(&valid, &too_many);
check_pos.current = valid;
Fragment(too_many) + Goto(nsm);
Fragment(too_many) + Goto(info.throw_no_such_method);
check_pos += Goto(join_after_optional);
}
@ -2208,90 +2232,114 @@ Fragment FlowGraphBuilder::BuildClosureCallArgumentsValidCheck(
Fragment check_named(has_named);
TargetEntryInstr *same, *different;
check_named += LoadLocal(vars->num_fixed_params);
check_named += IntConstant(descriptor.PositionalCount());
check_named += LoadLocal(info.num_fixed_params);
check_named += IntConstant(info.descriptor.PositionalCount());
check_named += BranchIfEqual(&same, &different);
check_named.current = same;
Fragment(different) + Goto(nsm);
Fragment(different) + Goto(info.throw_no_such_method);
if (descriptor.NamedCount() > 0) {
check_named += IntConstant(descriptor.NamedCount());
check_named += LoadLocal(vars->num_opt_params);
if (info.descriptor.NamedCount() > 0) {
check_named += IntConstant(info.descriptor.NamedCount());
check_named += LoadLocal(info.num_opt_params);
check_named += SmiRelationalOp(Token::kLTE);
TargetEntryInstr *valid, *too_many;
check_named += BranchIfTrue(&valid, &too_many);
check_named.current = valid;
Fragment(too_many) + Goto(nsm);
Fragment(too_many) + Goto(info.throw_no_such_method);
}
check_named += BuildClosureCallHasRequiredNamedArgumentsCheck(closure, nsm);
// Check the names for optional arguments. If applicable, also check that all
// required named parameters are provided.
check_named += BuildClosureCallNamedArgumentsCheck(info);
check_named += Goto(join_after_optional);
check_entry.current = join_after_optional;
return check_entry;
}
Fragment FlowGraphBuilder::BuildClosureCallNamedArgumentCheck(
LocalVariable* closure,
intptr_t pos,
JoinEntryInstr* nsm) {
auto const vars = parsed_function_->dynamic_closure_call_vars();
ASSERT(vars != nullptr);
ASSERT(has_saved_args_desc_array());
const ArgumentsDescriptor descriptor(saved_args_desc_array());
Fragment FlowGraphBuilder::BuildDynamicClosureCallChecks(
LocalVariable* closure) {
ClosureCallInfo info(closure, BuildThrowNoSuchMethod(),
saved_args_desc_array(),
parsed_function_->dynamic_closure_call_vars());
// If this isn't a named argument, then don't build anything.
if (pos < descriptor.PositionalCount()) return Fragment();
const intptr_t named_pos = pos - descriptor.PositionalCount();
ASSERT(named_pos < descriptor.NamedCount());
// We extract all the packed fields here so code generation that puts unboxed
// integers on the expression stack even in unoptimized code is in one place.
auto const rep = Slot::Function_packed_fields().representation();
// Loop over the indexes of the named parameters of the function, checking
// whether the named parameter at that index is required. If it is, then
// check whether it matches any of the names in the ArgumentsDescriptor.
JoinEntryInstr* loop = BuildJoinEntry();
Fragment body;
body += LoadLocal(info.closure);
body += LoadNativeField(Slot::Closure_function());
info.function = MakeTemporary("function");
// We iterate from [0, num_named), not [num_fixed, num_named) because the
// flag mask and index is based off the named index, not the param index.
Fragment check_arg_name;
check_arg_name += LoadLocal(vars->num_fixed_params);
check_arg_name +=
StoreLocal(TokenPosition::kNoSource, vars->current_param_index);
check_arg_name += Drop();
check_arg_name += Goto(loop);
body += LoadLocal(info.function);
body += LoadNativeField(Slot::Function_packed_fields());
body +=
BuildExtractPackedFieldIntoSmi<Function::PackedNumFixedParameters>(rep);
info.num_fixed_params = MakeTemporary("num_fixed_params");
Fragment loop_check(loop);
loop_check += LoadLocal(vars->current_param_index);
loop_check += LoadLocal(vars->num_max_params);
loop_check += SmiRelationalOp(Token::kLT);
TargetEntryInstr *no_more, *more;
loop_check += BranchIfTrue(&more, &no_more);
body += LoadLocal(info.function);
body += LoadNativeField(Slot::Function_packed_fields());
body += BuildExtractPackedFieldIntoSmi<Function::PackedNumOptionalParameters>(
rep);
info.num_opt_params = MakeTemporary("num_opt_params");
JoinEntryInstr* done = BuildJoinEntry();
// None of the parameter names matched.
Fragment(no_more) + Goto(nsm);
body += LoadLocal(info.num_fixed_params);
body += LoadLocal(info.num_opt_params);
body += SmiBinaryOp(Token::kADD);
info.num_max_params = MakeTemporary("num_max_params");
Fragment loop_body(more);
loop_body += LoadLocal(vars->parameter_names);
loop_body += LoadLocal(vars->current_param_index);
loop_body += LoadIndexed(compiler::target::kWordSize);
loop_body += Constant(String::ZoneHandle(Z, descriptor.NameAt(named_pos)));
TargetEntryInstr *str_equal, *str_neq;
loop_body += BranchIfEqual(&str_equal, &str_neq);
body += LoadLocal(info.function);
body += LoadNativeField(Slot::Function_packed_fields());
body += BuildExtractPackedFieldIntoSmi<
Function::PackedHasNamedOptionalParameters>(rep);
body += IntConstant(0);
body += StrictCompare(Token::kNE_STRICT);
info.has_named_params = MakeTemporary("has_named_params");
Fragment(str_equal) + Goto(done);
if (I->null_safety() || info.descriptor.NamedCount() > 0) {
body += LoadLocal(info.function);
body += LoadNativeField(Slot::Function_parameter_names());
info.parameter_names = MakeTemporary("parameter_names");
}
// Increment the index and jump back to the loop check.
loop_body.current = str_neq;
loop_body += LoadLocal(vars->current_param_index);
loop_body += IntConstant(1);
loop_body += SmiBinaryOp(Token::kADD, /*is_truncating=*/true);
loop_body += StoreLocal(TokenPosition::kNoSource, vars->current_param_index);
loop_body += Drop();
loop_body += Goto(loop);
if (info.descriptor.TypeArgsLen() > 0) {
body += LoadLocal(info.function);
body += LoadNativeField(Slot::Function_type_parameters());
info.type_parameters = MakeTemporary("type_parameters");
}
check_arg_name.current = done;
return check_arg_name;
// Check that the shape of the arguments generally matches what the
// closure function expects. The only remaining non-type check after this
// is that the names for optional arguments are valid.
body += BuildClosureCallArgumentsValidCheck(info);
// TODO(dartbug.com/40813): Move checks that are currently compiled
// in the closure body to here, using the dynamic versions of
// AssertSubtype to typecheck the type arguments using the runtime types
// available in the closure object.
// TODO(dartbug.com/40813): Move checks that are currently compiled
// in the closure body to here, using the dynamic versions of
// AssertAssignable to typecheck the parameters using the runtime types
// available in the closure object.
// Drop all the read-only temporaries at the end of the fragment.
if (info.type_parameters != nullptr) {
body += DropTemporary(&info.type_parameters);
}
if (info.parameter_names != nullptr) {
body += DropTemporary(&info.parameter_names);
}
body += DropTemporary(&info.has_named_params);
body += DropTemporary(&info.num_max_params);
body += DropTemporary(&info.num_opt_params);
body += DropTemporary(&info.num_fixed_params);
body += DropTemporary(&info.function);
return body;
}
FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher(
@ -2334,7 +2382,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher(
// Build any dynamic closure call checks before pushing arguments to the
// final call on the stack to make debugging easier.
LocalVariable* closure = NULL;
LocalVariable* closure = nullptr;
if (is_closure_call) {
closure = parsed_function_->ParameterVariable(0);
if (is_dynamic_call) {
@ -2344,30 +2392,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher(
InlineBailout(
"kernel::FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher");
// Init the variables we'll be using for dynamic call checking.
body += BuildDynamicCallVarsInit(closure);
JoinEntryInstr* nsm = BuildThrowNoSuchMethod();
// Check that the shape of the arguments generally matches what the
// closure function expects. The only remaining non-type check after this
// is that the names for optional arguments are valid.
body += BuildClosureCallArgumentsValidCheck(closure, nsm);
// TODO(dartbug.com/40813): Move checks that are currently compiled
// in the closure body to here, using the dynamic versions of
// AssertSubtype to typecheck the type arguments using the runtime types
// available in the closure object.
// TODO(dartbug.com/40813): Move checks that are currently compiled
// in the closure body to here, using the dynamic versions of
// AssertAssignable to typecheck the parameters using the runtime types
// available in the closure object.
//
// For now, we check that any named arguments have valid names.
for (intptr_t pos = descriptor.PositionalCount();
pos < descriptor.Count(); pos++) {
body += BuildClosureCallNamedArgumentCheck(closure, pos, nsm);
}
body += BuildDynamicClosureCallChecks(closure);
}
}

33
runtime/vm/compiler/frontend/kernel_to_il.h
View file

@ -80,31 +80,32 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
FlowGraph* BuildGraphOfMethodExtractor(const Function& method);
FlowGraph* BuildGraphOfNoSuchMethodDispatcher(const Function& function);
Fragment BuildDynamicCallVarsInit(LocalVariable* closure);
struct ClosureCallInfo;
// Tests whether the function parameter at the given index is required and
// branches to the appropriate fragment. Loads the parameter index to
// check from info.vars->current_param_index.
Fragment TestClosureFunctionNamedParameterRequired(
const ClosureCallInfo& info,
Fragment set,
Fragment not_set);
// The BuildClosureCall...Check methods differs from the checks built in the
// PrologueBuilder in that they are built for invoke field dispatchers,
// where the ArgumentsDescriptor is known at compile time but the specific
// closure function is retrieved at runtime.
// Builds checks that all required arguments are provided. Generates an empty
// fragment if null safety is not enabled.
Fragment BuildClosureCallHasRequiredNamedArgumentsCheck(
LocalVariable* closure,
JoinEntryInstr* nsm);
// Builds checks that the given named arguments have valid argument names
// and, in the case of null safe code, that all required named parameters
// are provided.
Fragment BuildClosureCallNamedArgumentsCheck(const ClosureCallInfo& info);
// Builds checks for checking the arguments of a call are valid for the
// function retrieved at runtime from the closure. Checks almost all the
// same cases as Function::AreArgumentsValid, leaving only name checking
// for optional named arguments to be checked during argument type checking.
Fragment BuildClosureCallArgumentsValidCheck(LocalVariable* closure,
JoinEntryInstr* nsm);
// function retrieved at runtime from the closure.
Fragment BuildClosureCallArgumentsValidCheck(const ClosureCallInfo& info);
// Builds checks that the given named argument has a valid argument name.
// Returns the empty fragment for positional arguments.
Fragment BuildClosureCallNamedArgumentCheck(LocalVariable* closure,
intptr_t pos,
JoinEntryInstr* nsm);
// Main entry point for building checks.
Fragment BuildDynamicClosureCallChecks(LocalVariable* closure);
FlowGraph* BuildGraphOfInvokeFieldDispatcher(const Function& function);
FlowGraph* BuildGraphOfFfiTrampoline(const Function& function);

1
runtime/vm/compiler/frontend/scope_builder.cc
View file

@ -441,7 +441,6 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() {
#define ADD_VAR(Name, _, __) scope_->AddVariable(vars->Name);
FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(ADD_VAR);
#undef ADD_VAR
needs_expr_temp_ = true;
}
FALL_THROUGH;
}

2
runtime/vm/parser.cc
View file

@ -345,8 +345,6 @@ ParsedFunction::EnsureDynamicClosureCallVars() {
if (dynamic_closure_call_vars_ != nullptr) return dynamic_closure_call_vars_;
dynamic_closure_call_vars_ = new (zone()) DynamicClosureCallVars();
const auto& type_Array = Type::ZoneHandle(zone(), Type::ArrayType());
const auto& type_Bool = Type::ZoneHandle(zone(), Type::BoolType());
const auto& type_Smi = Type::ZoneHandle(zone(), Type::SmiType());
#define INIT_FIELD(Name, TypeName, Symbol) \
dynamic_closure_call_vars_->Name = new (zone()) \

11
runtime/vm/parser.h
View file

@ -243,15 +243,12 @@ class ParsedFunction : public ZoneAllocated {
// method.
bool IsGenericCovariantImplParameter(intptr_t i) const;
// Variables needed for the InvokeFieldDispatcher for dynamic closure calls.
// Variables needed for the InvokeFieldDispatcher for dynamic closure calls,
// because they are both read and written to by the builders.
struct DynamicClosureCallVars : ZoneAllocated {
#define FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(V) \
V(current_param_index, Smi, CurrentParamIndex) \
V(has_named_params, Bool, HasNamed) \
V(num_fixed_params, Smi, NumFixed) \
V(num_opt_params, Smi, NumOpt) \
V(num_max_params, Smi, MaxParams) \
V(parameter_names, Array, ParameterNames)
V(current_num_processed, Smi, CurrentNumProcessed) \
V(current_param_index, Smi, CurrentParamIndex)
#define DEFINE_FIELD(Name, _, __) LocalVariable* Name = nullptr;
FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(DEFINE_FIELD)

6
runtime/vm/symbols.h
View file

@ -108,12 +108,8 @@ class ObjectPointerVisitor;
V(Double, "double") \
V(Dynamic, "dynamic") \
V(DynamicCall, "dyn:call") \
V(DynamicCallCurrentNumProcessedVar, ":dyn_call_current_num_processed") \
V(DynamicCallCurrentParamIndexVar, ":dyn_call_current_param_index") \
V(DynamicCallHasNamedVar, ":dyn_call_has_named") \
V(DynamicCallMaxParamsVar, ":dyn_call_max_params") \
V(DynamicCallNumFixedVar, ":dyn_call_num_fixed") \
V(DynamicCallNumOptVar, ":dyn_call_num_opt") \
V(DynamicCallParameterNamesVar, ":dyn_call_parameter_names") \
V(DynamicPrefix, "dyn:") \
V(EntryPointsTemp, ":entry_points_temp") \
V(EqualOperator, "==") \

35
tests/language/parameter/named_with_conversions_test.dart
View file

@ -29,9 +29,9 @@ Validate(tag, a, b) {
}
class HasMethod {
int calls;
int calls = 0;
HasMethod() : calls = 0 {}
HasMethod();
foo(tag, [a = 10, b = 20]) {
calls += 1;
@ -46,7 +46,7 @@ class HasMethod {
class HasField {
int calls = 0;
var foo, foo2;
late final dynamic foo, foo2;
HasField() {
foo = makeFoo(this);
@ -71,16 +71,6 @@ class HasField {
}
class NamedParametersWithConversionsTest {
static checkException(thunk) {
bool threw = false;
try {
thunk();
} catch (e) {
threw = true;
}
Expect.isTrue(threw);
}
static testMethodCallSyntax(a) {
a.foo('');
a.foo('a', 111);
@ -92,10 +82,12 @@ class NamedParametersWithConversionsTest {
Expect.equals(7, a.calls);
checkException(() => a.foo()); // Too few arguments.
checkException(() => a.foo('abc', 1, 2, 3)); // Too many arguments.
checkException(() => a.foo2('c', c: 1)); // Bad name.
checkException(() => a.foo2('c', a: 111, c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(() => a.foo()); // Too few arguments.
Expect.throwsNoSuchMethodError(
() => a.foo('abc', 1, 2, 3)); // Too many arguments.
Expect.throwsNoSuchMethodError(() => a.foo2('c', c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(
() => a.foo2('c', a: 111, c: 1)); // Bad name.
Expect.equals(7, a.calls);
}
@ -113,10 +105,11 @@ class NamedParametersWithConversionsTest {
Expect.equals(7, a.calls);
checkException(() => f()); // Too few arguments.
checkException(() => f('abc', 1, 2, 3)); // Too many arguments.
checkException(() => f2('c', c: 1)); // Bad name.
checkException(() => f2('c', a: 111, c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(() => f()); // Too few arguments.
Expect.throwsNoSuchMethodError(
() => f('abc', 1, 2, 3)); // Too many arguments.
Expect.throwsNoSuchMethodError(() => f2('c', c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(() => f2('c', a: 111, c: 1)); // Bad name.
Expect.equals(7, a.calls);
}

38
tests/language_2/parameter/named_with_conversions_test.dart
View file

@ -29,9 +29,9 @@ Validate(tag, a, b) {
}
class HasMethod {
int calls;
int calls = 0;
HasMethod() : calls = 0 {}
HasMethod();
foo(tag, [a = 10, b = 20]) {
calls += 1;
@ -45,11 +45,10 @@ class HasMethod {
}
class HasField {
int calls;
var foo, foo2;
int calls = 0;
dynamic foo, foo2;
HasField() {
calls = 0;
foo = makeFoo(this);
foo2 = makeFoo2(this);
}
@ -72,16 +71,6 @@ class HasField {
}
class NamedParametersWithConversionsTest {
static checkException(thunk) {
bool threw = false;
try {
thunk();
} catch (e) {
threw = true;
}
Expect.isTrue(threw);
}
static testMethodCallSyntax(a) {
a.foo('');
a.foo('a', 111);
@ -93,10 +82,12 @@ class NamedParametersWithConversionsTest {
Expect.equals(7, a.calls);
checkException(() => a.foo()); // Too few arguments.
checkException(() => a.foo('abc', 1, 2, 3)); // Too many arguments.
checkException(() => a.foo2('c', c: 1)); // Bad name.
checkException(() => a.foo2('c', a: 111, c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(() => a.foo()); // Too few arguments.
Expect.throwsNoSuchMethodError(
() => a.foo('abc', 1, 2, 3)); // Too many arguments.
Expect.throwsNoSuchMethodError(() => a.foo2('c', c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(
() => a.foo2('c', a: 111, c: 1)); // Bad name.
Expect.equals(7, a.calls);
}
@ -114,10 +105,11 @@ class NamedParametersWithConversionsTest {
Expect.equals(7, a.calls);
checkException(() => f()); // Too few arguments.
checkException(() => f('abc', 1, 2, 3)); // Too many arguments.
checkException(() => f2('c', c: 1)); // Bad name.
checkException(() => f2('c', a: 111, c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(() => f()); // Too few arguments.
Expect.throwsNoSuchMethodError(
() => f('abc', 1, 2, 3)); // Too many arguments.
Expect.throwsNoSuchMethodError(() => f2('c', c: 1)); // Bad name.
Expect.throwsNoSuchMethodError(() => f2('c', a: 111, c: 1)); // Bad name.
Expect.equals(7, a.calls);
}