Skip to content

Added default as an expression in argument lists #15437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Zend/Optimizer/optimize_func_calls.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ static void zend_try_inline_call(zend_op_array *op_array, zend_op *fcall, zend_o
} while (i < func->op_array.num_args);
}

for (i = 0; i < op_array->last; i++) {
// Don't inline calls with explicit default arguments.
if (op_array->opcodes[i].opcode == ZEND_FETCH_DEFAULT_ARG) {
return;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned off-list, this should check only for opcodes within the current call (between fcall and opline), not the entire function. It would be nice to also skip nested functions, but that will require listing all init/do opcodes.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the note. I am still trying to figure out how to do this.

Copy link
Member

Choose a reason for hiding this comment

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

Store fcall in a local var, loop, and bump until reaching opline.

Copy link
Member

Choose a reason for hiding this comment

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

There is code for that in PHP already somewhere, and now also in Xdebug: xdebug/xdebug@551b203#diff-846d4897469701b1d805f3574922dc47b632fd189087a2589b2e01e848585a8bR378 — that should probably become a PHP API function.

Copy link
Author

Choose a reason for hiding this comment

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

I'll be honest, I still have no idea what to do here, and no idea how the Xdebug code relates to this. Anyway, I'll deal with this if the feature gets accepted.

Copy link
Member

Choose a reason for hiding this comment

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

for (zend_op *op = fcall + 1; op != opline; op++) {
	// Don't inline calls with explicit default arguments.
	if (op->opcode == ZEND_FETCH_DEFAULT_ARG) {
		return;
	}
}

Copy link
Author

@Bilge Bilge Aug 28, 2024

Choose a reason for hiding this comment

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

Thanks. Clearly I need to understand more about what oparrays, oplines and opcodes really are, and how they relate to each other. I tend to imagine that an oparray contains oplines which contain opcodes, but I still don't really know why oparray is even a thing, and when you would bundle oplines together.


if (RETURN_VALUE_USED(opline)) {
zval zv;

Expand Down
54 changes: 54 additions & 0 deletions Zend/tests/default_expression/default_expressions.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
--TEST--
Tests an exhaustive list of valid expressions containing the default keyword
--FILE--
<?php
function F($V = 2) { return $V; }

var_dump(F(default + 1));
var_dump(F(default - 1));
var_dump(F(default * 2));
var_dump(F(default / 2));
var_dump(F(default % 2));
var_dump(F(default & 1));
var_dump(F(default | 1));
var_dump(F(default ^ 2));
var_dump(F(default << 1));
var_dump(F(default >> 1));
var_dump(F(default ** 2));
echo PHP_EOL;
var_dump(F(1 + default));
var_dump(F(1 - default));
var_dump(F(2 * default));
var_dump(F(2 / default));
var_dump(F(2 % default));
var_dump(F(1 & default));
var_dump(F(1 | default));
var_dump(F(2 ^ default));
var_dump(F(1 << default));
var_dump(F(1 >> default));
var_dump(F(2 ** default));
?>
--EXPECT--
int(3)
int(1)
int(4)
int(1)
int(0)
int(0)
int(3)
int(0)
int(4)
int(1)
int(4)

int(3)
int(-1)
int(4)
int(1)
int(0)
int(0)
int(3)
int(0)
int(4)
int(0)
int(4)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--TEST--
Tests passing default to a required parameter of an internal class
--FILE--
<?php

DateTime::createFromInterface(default);
?>
--EXPECTF--
Fatal error: Uncaught ValueError: Cannot pass default to required parameter 1 of DateTime::createFromInterface() in %s:%d
Stack trace:%a
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--TEST--
Tests passing default to a required parameter of an internal function
--FILE--
<?php

json_encode(default);
?>
--EXPECTF--
Fatal error: Uncaught ValueError: Cannot pass default to required parameter 1 of json_encode() in %s:%d
Stack trace:%a
12 changes: 12 additions & 0 deletions Zend/tests/default_expression/global_user_function.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Tests passing default to a global user function parameter
--FILE--
<?php

function F($V = 'Alfa') {
var_dump($V);
}
F(default);
?>
--EXPECT--
string(4) "Alfa"
10 changes: 10 additions & 0 deletions Zend/tests/default_expression/internal_function.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--TEST--
Tests passing default to an internal function parameter
--FILE--
<?php

json_encode([], 0, $D = default);
var_dump($D);
?>
--EXPECT--
int(512)
9 changes: 9 additions & 0 deletions Zend/tests/default_expression/invalid_context.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
--TEST--
Tests specifying the default keyword in an invalid context throws a compile error
--FILE--
<?php

default;
?>
--EXPECTF--
Fatal error: Cannot use default in non-argument context. in %s on line %d
14 changes: 14 additions & 0 deletions Zend/tests/default_expression/multiple_arguments.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Test passing multiple default arguments to global user function parameters
--FILE--
<?php

function F($V = 'Alfa', $V2 = 'Bravo') {
var_dump($V);
var_dump($V2);
}
F(default, default);
?>
--EXPECT--
string(4) "Alfa"
string(5) "Bravo"
16 changes: 16 additions & 0 deletions Zend/tests/default_expression/named_arguments.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
Tests passing default as a named argument
--FILE--
<?php

json_encode([], depth: $D = default);
var_dump($D);

function F($V = 'Alfa', $V2 = 1) {
var_dump($V2);
}
F(V2: default + 1);
?>
--EXPECT--
int(512)
int(2)
12 changes: 12 additions & 0 deletions Zend/tests/default_expression/nested_calls.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Test passing default in a parent argument position that is less than the child call's total number of arguments
--FILE--
<?php

function F($x = 1, $y = 2) {
return $x + $y;
}
var_dump(F(F(0, 1) + default));
?>
--EXPECTF--
int(4)
12 changes: 12 additions & 0 deletions Zend/tests/default_expression/new_class_parameter.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Test passing default to a parameter with a new class as the default
--FILE--
<?php

function F($V = new stdClass) {}
F($D = default);
var_dump($D);
?>
--EXPECT--
object(stdClass)#1 (0) {
}
14 changes: 14 additions & 0 deletions Zend/tests/default_expression/user_class_method.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Tests passing default to a user class method parameter
--FILE--
<?php

class C {
function F($V = 'Alfa') {
var_dump($V);
}
}
new C()->f(default);
?>
--EXPECT--
string(4) "Alfa"
14 changes: 14 additions & 0 deletions Zend/tests/default_expression/user_class_static_method.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Tests passing default to a user class static method parameter
--FILE--
<?php

class C {
static function F($V = 'Alfa') {
var_dump($V);
}
}
C::f(default);
?>
--EXPECT--
string(4) "Alfa"
3 changes: 3 additions & 0 deletions Zend/zend_ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -2013,6 +2013,9 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
EMPTY_SWITCH_DEFAULT_CASE();
}
break;
case ZEND_AST_DEFAULT:
smart_str_appends(str, "default");
break;

/* 1 child node */
case ZEND_AST_VAR:
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ enum _zend_ast_kind {
ZEND_AST_TYPE,
ZEND_AST_CONSTANT_CLASS,
ZEND_AST_CALLABLE_CONVERT,
ZEND_AST_DEFAULT,

/* 1 child node */
ZEND_AST_VAR = 1 << ZEND_AST_NUM_CHILDREN_SHIFT,
Expand Down
48 changes: 35 additions & 13 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -3701,6 +3701,7 @@ static uint32_t zend_compile_args(
uint32_t i;
bool uses_arg_unpack = 0;
uint32_t arg_count = 0; /* number of arguments not including unpacks */
uint32_t prev_arg_num = CG(arg_num);

/* Whether named arguments are used syntactically, to enforce language level limitations.
* May not actually use named argument passing. */
Expand Down Expand Up @@ -3778,6 +3779,8 @@ static uint32_t zend_compile_args(
arg_count++;
}

CG(arg_num) = arg_num;

/* Treat passing of $GLOBALS the same as passing a call.
* This will error at runtime if the argument is by-ref. */
if (zend_is_call(arg) || is_globals_fetch(arg)) {
Expand Down Expand Up @@ -3892,10 +3895,24 @@ static uint32_t zend_compile_args(
zend_emit_op(NULL, ZEND_CHECK_UNDEF_ARGS, NULL, NULL);
}

CG(arg_num) = prev_arg_num;

return arg_count;
}
/* }}} */

static void zend_compile_default(znode *result, zend_ast *ast)
{
uint32_t arg_num = CG(arg_num);

if (arg_num == 0) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use default in non-argument context.");
}

zend_op *opline = zend_emit_op_tmp(result, ZEND_FETCH_DEFAULT_ARG, NULL, NULL);
opline->op1.num = arg_num;
}

ZEND_API uint8_t zend_get_call_op(const zend_op *init_op, zend_function *fbc) /* {{{ */
{
if (fbc) {
Expand Down Expand Up @@ -6350,11 +6367,12 @@ static uint32_t count_match_conds(zend_ast_list *arms)

for (uint32_t i = 0; i < arms->children; i++) {
zend_ast *arm_ast = arms->child[i];
if (arm_ast->child[0] == NULL) {
zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);

if (conds->child[0]->kind == ZEND_AST_DEFAULT) {
continue;
}

zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);
num_conds += conds->children;
}

Expand All @@ -6364,12 +6382,13 @@ static uint32_t count_match_conds(zend_ast_list *arms)
static bool can_match_use_jumptable(zend_ast_list *arms) {
for (uint32_t i = 0; i < arms->children; i++) {
zend_ast *arm_ast = arms->child[i];
if (!arm_ast->child[0]) {
zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);

if (conds->child[0]->kind == ZEND_AST_DEFAULT) {
/* Skip default arm */
continue;
}

zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);
for (uint32_t j = 0; j < conds->children; j++) {
zend_ast **cond_ast = &conds->child[j];

Expand Down Expand Up @@ -6410,8 +6429,9 @@ static void zend_compile_match(znode *result, zend_ast *ast)

for (uint32_t i = 0; i < arms->children; ++i) {
zend_ast *arm_ast = arms->child[i];
zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);

if (!arm_ast->child[0]) {
if (conds->child[0]->kind == ZEND_AST_DEFAULT) {
if (has_default_arm) {
CG(zend_lineno) = arm_ast->lineno;
zend_error_noreturn(E_COMPILE_ERROR,
Expand Down Expand Up @@ -6439,15 +6459,15 @@ static void zend_compile_match(znode *result, zend_ast *ast)
uint32_t cond_count = 0;
for (uint32_t i = 0; i < arms->children; ++i) {
zend_ast *arm_ast = arms->child[i];

if (!arm_ast->child[0]) {
continue;
}

zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);

for (uint32_t j = 0; j < conds->children; j++) {
zend_ast *cond_ast = conds->child[j];

if (conds->child[0]->kind == ZEND_AST_DEFAULT) {
break;
}

znode cond_node;
zend_compile_expr(&cond_node, cond_ast);

Expand Down Expand Up @@ -6499,10 +6519,9 @@ static void zend_compile_match(znode *result, zend_ast *ast)
for (uint32_t i = 0; i < arms->children; ++i) {
zend_ast *arm_ast = arms->child[i];
zend_ast *body_ast = arm_ast->child[1];
zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);

if (arm_ast->child[0] != NULL) {
zend_ast_list *conds = zend_ast_get_list(arm_ast->child[0]);

if (conds->child[0]->kind != ZEND_AST_DEFAULT) {
for (uint32_t j = 0; j < conds->children; j++) {
zend_ast *cond_ast = conds->child[j];

Expand Down Expand Up @@ -11497,6 +11516,9 @@ static void zend_compile_expr_inner(znode *result, zend_ast *ast) /* {{{ */
case ZEND_AST_MATCH:
zend_compile_match(result, ast);
return;
case ZEND_AST_DEFAULT:
zend_compile_default(result, ast);
return;
default:
ZEND_ASSERT(0 /* not supported */);
}
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include "zend_system_id.h"
#include "zend_call_stack.h"
#include "zend_attributes.h"
#include "ext/reflection/php_reflection.h"
#include "Optimizer/zend_func_info.h"

/* Virtual current working directory support */
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend_globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ struct _zend_compiler_globals {
uint32_t rtd_key_counter;

zend_stack short_circuiting_opnums;

// Current 1-based argument index if compiling arguments, otherwise zero.
uint32_t arg_num;
#ifdef ZTS
uint32_t copied_functions_count;
#endif
Expand Down
Loading
Loading