Skip to content

Named args#853

Open
mkornaukhov wants to merge 13 commits intomasterfrom
mkornaukhov03/named-args
Open

Named args#853
mkornaukhov wants to merge 13 commits intomasterfrom
mkornaukhov03/named-args

Conversation

@mkornaukhov
Copy link
Contributor

@mkornaukhov mkornaukhov commented Jun 16, 2023

@mkornaukhov mkornaukhov force-pushed the mkornaukhov03/named-args branch from 8c78983 to cdcbb9f Compare June 26, 2023 16:19
@Tsygankov-Slava Tsygankov-Slava mentioned this pull request Jul 3, 2023
44 tasks
@mkornaukhov mkornaukhov changed the title [WIP] Named args Named args Jul 4, 2023
if (cur->type() == tok_colon) {
puts("!!!!!!!");
next_cur();
VertexPtr value = get_expression();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably add kphp_error (...) here.

VertexPtr value = get_expression();
kphp_error (value, "...");

// name.debugPrint();
// assert(name->type() == op_func_name);
if (cur->type() == tok_colon) {
puts("!!!!!!!");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why's it here?

};

// at first, convert f(1, ...[2, ...[3]], ...$all, ...[5]) to f(1,2,3,...$all,5)
std::vector<VertexPtr> flatten_call_args(VertexRange args, VertexRange func_params) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that here you can pass arguments by reference and make them const.

std::vector<VertexPtr> flatten_call_args(const VertexRange& args, const VertexRange& func_params) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VertexRange is just a pair of two pointers, so I think it's better to pass-by-value than pass-by-reference

@@ -104,21 +119,29 @@ VertexAdaptor<op_func_call> CheckFuncCallsAndVarargPass::process_varargs(VertexA
variadic_args_passed.emplace_back(VertexUtil::create_conv_to(tp_array, unpack_as_varg->array()));
needs_wrap_array_merge = true;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be simplified.

if (...) {
    is_just_single_arg_forwarded = true;
} else {
    needs_wrap_array_merge = true;
}
variadic_args_passed.emplace_back(VertexUtil::create_conv_to(tp_array, unpack_as_varg->array()));

int vararg_idx = call_arg_idx;
while (call_arg_idx < call_args.size()) {
call_arg_to_func_param[call_arg_idx] = f_called_params[vararg_idx].as<op_func_param>();
call_arg_idx++;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's better to use prefix addition here? Why do we need an extra copy?

++call_arg_idx;

Below also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do mapping call arg --> func arg to call patch_call_arg_on_func_call on corresponding expressions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PHP8 PHP8 feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants