Bind traits before parent class #15878
Conversation
|
A couple of weeks ago I would have suggested to apply this master only, but now I'm unsure whether it should still go into PHP-8.4. Maybe the @php/release-managers-84 want to have a look. |
|
I'm also unsure about this myself. This late and this ambiguous, I think it'd be better off in 8.5. |
|
Can you explain what you mean by ambiguous? It might be good to adjust methods too, which should make behavior consistent and predictable. But it might require a bit more changes. |
|
I was referring to Dmitry's comment about possible fallout. |
|
Ok, thanks for the clarification. I will make the changes for methods, and then target this to master. |
iluuu1994
changed the title
This more accurately matches the "copy & paste" semantics described in the documentation. Abstract trait methods diverge from this behavior, given that a parent method can satisfy trait methods used in the child. In that case, the method is not copied, but the check is performed after the parent has been bound. Fixes phpGH-15753 Fixes phpGH-16198
zend_add_trait_method() never overrides a parent method, given that parents are not bound yet. Instead, compatibility is checked in do_inherit_method(), as per usual.
There was a problem hiding this comment.
I can't say I fully understand the implementation, due to my lack of familiarity with the code. The test changes however make sense to me, so this definitely is a LGTM in concept.
I'm particularly happy about the workaround in d344fe0 going away.
I've some minor inline comments about the implementation.
|
For reference, #14148 is somewhat related to this as well. |
This more accurately matches the "copy & paste" semantics described in
the documentation. Abstract trait methods diverge from this behavior,
given that a parent method can satisfy trait methods used in the child.
In that case, the method is not copied, but the check is performed after
the parent has been bound.
Fixes GH-15753
Fixes GH-16198