php / php-src

The PHP Interpreter
https://www.php.net
Other
37.58k stars 7.7k forks source link

Why are constant expressions decided at compile time instead of in the grammar? #14747

Closed chx closed 1 week ago

chx commented 1 week ago

Description

I was looking at zend_language_parser.y and I was surprised to see attribute allows a full expr and there's no const_expr definition. When I look at zend_is_allowed_in_const_expr on the other hand, it only contains multiple comparisons to constants defined in the _zend_ast_kind enum. In turn, it seems to me these are created in zend_language_parser.y using calls like zend_ast_create(ZEND_AST_CLASS_CONST, $1, $4); That means it should be possible to simply take all the declarations which lead to such an zend_ast_create() calls for every constant in zend_is_allowed_in_const_expr and thus define a const_expr in the language parser instead of deciding whether an expression is constant at compile time.

Mind you I never worked with yacc before so I might be totally wrong here, I am just reading the source code. But if my observation is correct... why there isn't a const_expr? Should I make an attempt in creating one :) ? Is there tooling which would help creating one such?

PHP Version

PHP 8.3.8

Operating System

No response

iluuu1994 commented 1 week ago

Two reasons mainly: It would require duplicating parts of the grammar, given that expressions are recursive, and handling it this way allows us to provide better error messages. I'm not sure the resulting grammar would be cleaner.

chx commented 1 week ago

I thought it would just require splitting some definitions and moving parts into new declarations but I do not readily see what would get duplicated.

iluuu1994 commented 1 week ago

How? For example:

expr:
    ...
    |   expr '+' expr   { ... }
;

How do you separate this into expressions and constant expressions? The following does not work:

const_expr:
    ...
    |   const_expr '+' const_expr { ... }
;

expr:
        const_expr {}
    |   ...
;

because then the lhs/rhs of + cannot contain other exprs, only const_exprs. Which means you'll need to duplicate all productions.

chx commented 1 week ago

Thanks, I guess then it's not possible :(