Make ASSIGN, ASSIGN_OP, INC and DEC opcodes to return IS_TMP_VAR instead of IS_VAR #5158
Conversation
|
Looks fine to me. Please add an UPGRADING note about the notice->exception change. There's also an unrelated change to |
|
As this further reduces the number of Not sure if this really makes sense though... |
I will do.
Yeah, already noticed. Seems, Remi added it by mistake. |
We have to keep IS_VAR results, in cases where IS_REFERENCE is possible (or expected) e.g. DO_FCALL, ASSIGN_REF. We probably may use IS_TMP_VAR for most of DO_UCALL/DO_ICALL. One other related idea - change the remaining "Only variables should be passed by reference" notices into exceptions "Cannot pass parameter %d by reference". |
The idea is to allow both IS_VAR and IS_TMP_VAR result types for the remaining opcodes. Whether IS_VAR or IS_TMP_VAR is used would depend on whether the opcode is in W or R context. Then the opcode would be responsible itself for performing a dereference if the result type is IS_TMP_VAR. That is, we move responsibility for performing the dereference from the user opcode to the producer opcode. This did not make sense before, as there were many opcodes that could produce references, but now this might make more sense.
Yes, this probably makes sense, to avoid breakages when things switch between IS_VAR <-> IS_TMP_VAR. |
Go it, but not sure if this makes sense, because we will always check for IS_REFERENCE in that opcodes. Now we check only on use, and most opcodes optimized to check the most probable type first. Anyway, it makes sense to think a bit more.
I'll prepare a separate PR later. |
…ead of IS_VAR. This helps to avoid unnecessary IS_REFERENCE checks. This changes some notices "Only variables should be passed by reference" to exception "Cannot pass parameter %d by reference". Also, for consistency, compile-time fatal error "Only variables can be passed by reference" was converted to exception "Cannot pass parameter %d by reference"
dstogov
force-pushed
the
result_tmp_var
branch
from
3668da1 to
0824c3d
Compare
| Actually, all assignments (ZEND_ASSIGN, ZEND_ASSIGN_DIM, ZEND_ASSIGN_OBJ, | ||
| ZEND_ASSIGN_STATIC_PROP), all compound assignments (ZEND_ASSIGN_OP, | ||
| ZEND_ASSIGN_DIM_OP, ZEND_ASSIGN_OBJ_OP, ZEND_ASSIGN_STATIC_PROP_OP) and all | ||
| pre increments/decremets (ZEND_PRE_INC, ZEND_PRE_DEC, ZEND_PRE_INC_OBJ |
|
Merged as 64b40f6 |
This helps to avoid unnecessary IS_REFERENCE checks.
This changes some notices "Only variables should be passed by reference" to exception "Cannot pass parameter %d by reference".
Also, for consistency, compile-time fatal error "Only variables can be passed by reference" was converted to exception "Cannot pass parameter %d by reference"