Preserve the original AST for closures in const-expressions #17120
Conversation
|
Tbh, I would prefer a simple solution, e.g. by printing a placeholder in the ast dumper. Accurate ast dumping is not very important. |
|
That would also be fine with me. Nevertheless I would suggest to keep the first commit (I can send a separate PR for that), as it it definitely fixes a bug in handling of |
This comment was marked as off-topic.
This comment was marked as off-topic.
Previously `zend_ast_decl` AST nodes were handled as if they were regular `zend_ast` nodes. This was not observable, because copying an AST only happened when evaluating const-expressions, which could not include declarations until the “Support Closures in constant expressions” RFC.
This is necessary when stringifying a `ReflectionAttribute` that has a closure as a parameter.
TimWolla
force-pushed
the
ast-op-array-original-ast
branch
from
05e868f to
45057db
Compare
|
@iluuu1994 I actually like the full AST being there. While it's not fundamentally necessary for usability, it's nice to have I think. |
|
One difference imo is that closures may grow large, in comparison to singular expressions. This ast is almost never used, but will still take up shm space with little benefit. |
|
@iluuu1994 I doubt the amount of closures within attributes will be significant enough to warrant considerations in that regard. |
Fixes GH-17096.