Repository navigation
Conversation
The original PR phpGH-16952 introduced special handling and a special flag to store the attribute via an OP_DATA opcode. It required special handling in various places. This commit reworks that by introducing a type specialization of the zend_constant structure and storing the attribute data in the main op array. This reduces complexity and removes special handling in various places.
Girgias
left a comment
There was a problem hiding this comment.
I did an extremely cursory read and can't see anything obviously wrong, but then it's 3 AM after me going out, so you know.
Girgias
left a comment
There was a problem hiding this comment.
Had a better look and I can't see any problems and this is a nice simplification. :)
| uint32_t lineno; | ||
| /* Parameter offsets start at 1, everything else uses 0. */ | ||
| /* Parameter offsets start at 1. | ||
| * User constants use their own offset in the shared table of the main op_array (see zend_attributed_constant). |
There was a problem hiding this comment.
a bit confused here - for parameters, this is saying that the current attribute applies to parameter #n in a function
for user constants, I'm reading this as saying that the current attribute applies to the constant at offset #n in the main op_array - is that correct? Can you improve the documentation a bit?
There was a problem hiding this comment.
Almost, I've updated the comment with more explanation
DanielEScherzer
left a comment
There was a problem hiding this comment.
okay for ext/reflection
For the rest, mostly makes sense, but I'm not confident enough to approve that part since this touches some stuff I don't really understand
The original PR GH-16952 introduced special handling and a special flag to store the attribute via an OP_DATA opcode. It required special handling in various places.
This commit reworks that by introducing a type specialization of the zend_constant structure and storing the attribute data in the main op array.
This reduces complexity and removes special handling in various places.