Skip to content

Fix enum case conflict in trait binding#21771

Open
prateekbhujel wants to merge 2 commits intophp:PHP-8.4from
prateekbhujel:prateekbhujel/fix-gh-21760-enum-trait-const-conflict
Open

Fix enum case conflict in trait binding#21771
prateekbhujel wants to merge 2 commits intophp:PHP-8.4from
prateekbhujel:prateekbhujel/fix-gh-21760-enum-trait-const-conflict

Conversation

@prateekbhujel
Copy link
Copy Markdown
Contributor

Fixes #21760.

Trait constant binding currently assumes that an existing entry in the constants table is a normal class constant. If the existing entry is an enum case with the same name, that path crashes instead of rejecting the conflict cleanly.

Reject that conflict up front during trait constant binding and cover it with a regression test.

Copy link
Copy Markdown
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.

This also needs to target 8.4 as a bug fix :) (rebase localy and force push before changing the branch on GH)

Comment thread Zend/zend_inheritance.c Outdated
@prateekbhujel
Copy link
Copy Markdown
Contributor Author

Updated this to target PHP-8.4 and retargeted the PR there.

@prateekbhujel prateekbhujel changed the base branch from master to PHP-8.4 April 17, 2026 00:12
@prateekbhujel prateekbhujel requested a review from Girgias April 17, 2026 00:13
Copy link
Copy Markdown
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.

LGTM but maybe @TimWolla has some opinion on the wording of the error message.

@prateekbhujel
Copy link
Copy Markdown
Contributor Author

Sounds good, I’ll leave it as is for now and can adjust the wording if @TimWolla has a preference.

Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

LGTM otherwise, thanks!

Comment thread Zend/zend_inheritance.c
const zend_class_entry *ce, const zend_class_constant *trait_constant, zend_string *name
) {
zend_error_noreturn(E_COMPILE_ERROR,
"Cannot use trait %s, because constant %s conflicts with enum case %s::%s",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"Cannot use trait %s, because constant %s conflicts with enum case %s::%s",
"Cannot use trait constant %s::%s as it conflicts with enum case %s::%s",

sounds a bit less clunky.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, the trait is what is being used, not the constant. But I agree with part of the suggestion.

Mine would be:

Cannot use trait X, because X::Up conflicts with enum case Direction::Up

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@TimWolla Your proposal sounds good.

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.

Trait with class constant name conflict against enum case causes SEGV

5 participants