Skip to content

Zend: Fix anonymous closure names #19011

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

Merged
merged 2 commits into from
Jul 3, 2025
Merged

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jul 2, 2025

Noticed this when looking at the test output of #18932 where the anonymous functions had a name of Closure::_invoke instead of the better names that @TimWolla introduced.

@DanielEScherzer can you pull this patch for the PR to see what the test messages are?

@Girgias Girgias force-pushed the closure-name-fix branch from a1ba52d to c889994 Compare July 2, 2025 18:57
@Girgias Girgias requested a review from arnaud-lb July 2, 2025 19:03
@Girgias Girgias marked this pull request as ready for review July 2, 2025 20:02
@Girgias Girgias requested a review from dstogov as a code owner July 2, 2025 20:02
@TimWolla
Copy link
Member

TimWolla commented Jul 2, 2025

Related: #18063.

Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

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

This looks good to me in general.

Regarding PFAs: With the patch applied, zend_get_callable_name_ex() returns the underlying function name (without scope). I will adjust this in the PFA branch.

Co-authored-by: Arnaud Le Blanc <[email protected]>
@Girgias Girgias merged commit f91f80c into php:master Jul 3, 2025
1 check passed
@Girgias Girgias deleted the closure-name-fix branch July 3, 2025 11:01
@DanielEScherzer
Copy link
Member

Sorry I wasn't able to review this in time, I'll update #18932 with the results

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.

4 participants