Skip to content

zend_ast: Surround function by parens when exporting calls to function stored in property#22376

Open
TimWolla wants to merge 1 commit into
php:masterfrom
TimWolla:ast-print-prop-call
Open

zend_ast: Surround function by parens when exporting calls to function stored in property#22376
TimWolla wants to merge 1 commit into
php:masterfrom
TimWolla:ast-print-prop-call

Conversation

@TimWolla

Copy link
Copy Markdown
Member

The extra parentheses are needed to disambiguate method calls from calls to a function stored in a property.

Fixes #22373.

…n stored in property

The extra parentheses are needed to disambiguate method calls from calls to a
function stored in a property.

Fixes php#22373.
Comment thread Zend/zend_ast.c
case ZEND_AST_CALL: {
zend_ast *left = ast->child[0];
if (left->kind == ZEND_AST_ARROW_FUNC || left->kind == ZEND_AST_CLOSURE) {
if (left->kind == ZEND_AST_ARROW_FUNC || left->kind == ZEND_AST_CLOSURE || left->kind == ZEND_AST_PROP) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This only covers ZEND_AST_PROP. ZEND_AST_NULLSAFE_PROP and ZEND_AST_STATIC_PROP callees drop their parens the same way: ($o?->f)() exports as $o?->f() (nullsafe method call) and (C::$sf)() as C::$sf() (variable static method call), neither of which round-trips. Worth folding both into the condition and the test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good points, thank you. Perhaps it makes sense to invert the condition: Always add parens, unless it's known to be a bare identifier.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AST pretty-printing drops meaningful parentheses surrounding property access

2 participants