Skip to content

Don't substitute self/parent for anonymous class #19537

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iluuu1994
Copy link
Member

Fixes GH-18373

@iluuu1994 iluuu1994 requested a review from a team August 20, 2025 21:24
@iluuu1994 iluuu1994 changed the title Dont substitute self/parent for anonymous class Don't substitute self/parent for anonymous class Aug 20, 2025
@@ -7089,18 +7089,21 @@ static zend_type zend_compile_single_typename(zend_ast *ast)
zend_assert_valid_class_name(class_name, "a type name");
} else {
ZEND_ASSERT(fetch_type == ZEND_FETCH_CLASS_SELF || fetch_type == ZEND_FETCH_CLASS_PARENT);
bool substitute_self_parent = zend_is_scope_known()
&& CG(active_class_entry)
Copy link
Member

@nielsdos nielsdos Aug 23, 2025

Choose a reason for hiding this comment

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

Isn't this always true? (at this point that is)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet. It was either this condition, or duplicating the check in the self and parent branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify, no scope is considered a known scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, actually given the assert that doesn't make sense. I think it would have to be repeated either way. I thought I tried removing it but got a NULL pointer. I'll try again before merging.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this, this had slipped my mind again.

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.

Change in class name resolution for self, parent and static
3 participants