ext/gmp: Fix some issues regarding operator overloading #16015
Conversation
Girgias
force-pushed
the
gmp-8.2-null-op-overloading
branch
from
861caf5 to
fadd51d
Compare
| if (UNEXPECTED(!IS_GMP(op2))) { | ||
| goto typeof_op_failure; | ||
| } else { | ||
| // TODO We shouldn't cast the GMP object to int here |
|
Isn't this too large a BC break for targeting PHP-8.2? Some operators now behave stricter than calling the functions in coercive typing mode (see e.g. https://3v4l.org/nQkCK). |
|
For safety best merge this in 8.4 and up then? |
Okay, let me see if I can handle the string case. |
|
Right I remember part of it... 8.2 doesn't have the |
And improve the behaviour for other not supported types
…ed operator" This reverts commit 9cea1a2.
Girgias
force-pushed
the
gmp-8.2-null-op-overloading
branch
from
6f8c6e8 to
be6bded
Compare
|
@cmb69 hopefully I've covered all the cases now. |
There was a problem hiding this comment.
This is gonna be as good as it's gonna get.
Note that I'm not convinced of your UNEXPECTED macro usage. UNEXPECTED actively penalizes code by moving it to the cold section, this is different than doing e.g. !EXPECTED. I try to use the UNEXPECTED macro only on error cases. Anyway, it's not that big of a deal.
Related to #15660
The behaviour of the engine in regards to operator overloading atm is kinda bonkers and needs some deeper fixes.
The implementation of operator overloading in GMP also needs an overhaul as it should return
FAILUREinstead of throwing exceptions if it cannot handle the operands.