Skip to content

zend_test: Fix function and constant imports in compile_to_ast and add support for grouping - #24418

Open
mainframe98 wants to merge 1 commit into
php:PHP-8.4from
mainframe98:zend-test-compile-to-ast-grouped-use-support
Open

zend_test: Fix function and constant imports in compile_to_ast and add support for grouping#24418
mainframe98 wants to merge 1 commit into
php:PHP-8.4from
mainframe98:zend-test-compile-to-ast-grouped-use-support

Conversation

@mainframe98

@mainframe98 mainframe98 commented Oct 18, 2026 •
edited
Loading

Copy link
Copy Markdown

While writing a test for it, I discovered the logic for printing
the AST representation of use statements was broken and grouping
did not work. That has been fixed and incorporated into the test.

@mainframe98
mainframe98 force-pushed the zend-test-compile-to-ast-grouped-use-support branch from 29baf1b to c4c298b Compare October 19, 2026 17:21
@ndossche

ndossche commented Oct 20, 2026

Copy link
Copy Markdown
Member

So this is actually a bugfix, but your PR title doesn't imply that.
Can you please retitle your PR, and target branch PHP-8.4 please? (Since that's the lowest supported bugfix branch)

@mainframe98
mainframe98 force-pushed the zend-test-compile-to-ast-grouped-use-support branch from c4c298b to 782d13d Compare October 20, 2026 17:15
@mainframe98
mainframe98 changed the base branch from master to PHP-8.4 October 20, 2026 17:15
@mainframe98
mainframe98 force-pushed the zend-test-compile-to-ast-grouped-use-support branch from 782d13d to 56214fb Compare October 20, 2026 17:18
@mainframe98 mainframe98 changed the title zend_test: support grouped use statements in compile_to_ast zend_test: Fix function and constant imports in compile_to_ast and add support for grouping Oct 20, 2026
@mainframe98

mainframe98 commented Oct 20, 2026

Copy link
Copy Markdown
Author

So this is actually a bugfix, but your PR title doesn't imply that.

I wasn't sure if compile_to_ast even counts as a "public" API, so I figured targeting master was the safest bet. I hope this is better.

@mainframe98

mainframe98 commented Oct 20, 2026

Copy link
Copy Markdown
Author

I wasn't aware that zend_test_compile_to_ast is that new, but apparently it debuted in #16952, which is PHP 8.5 or newer. Should I split out the test into a separate PR targeting PHP 8.5 or just master?

@ndossche

ndossche commented Oct 22, 2026

Copy link
Copy Markdown
Member

Ah lovely, the function doesn't exist on PHP 8.4 indeed!
Since this is zend_test, i.e. an internal php-src-maintainer-only extension, I've just backported that zend_test function to PHP 8.4's zend_test. Can you please rebase? The test should then work 🙂

@mainframe98
zend_test: Fix function and constant imports in compile_to_ast and ad…
3d97f68 
…d support for grouping

While writing a test for it, I discovered the logic for printing
the AST representation of use statements was broken and grouping
did not work. That has been fixed and incorporated into the test.
@mainframe98
mainframe98 force-pushed the zend-test-compile-to-ast-grouped-use-support branch from 56214fb to 3d97f68 Compare October 22, 2026 19:12
mainframe98 pushed a commit to mainframe98/php-src that referenced this pull request Oct 22, 2026
@ndossche
zend_test: Backport zend_test_compile_to_ast() function
359ce15 
This was introduced in 8.5.
We need this in 8.4 too to debug AST dumping (see phpGH-24418).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Category: Engine Extension: zend_test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mainframe98 @ndossche