fix sv_numeq and add other SV numeric comparison APIs #23966
Conversation
|
Draft for now, I suspect the overloading support has problems when the current op context is void. |
tonycoz
force-pushed
the
23918-cmp-api
branch
2 times, most recently
from
13ed11e to
096a2ec
Compare
|
There was some discussion in IRC on how to handle overloading. In particular, amagic_call() will check if the I can see these and similar APIs being called in a few contexts:
I think the default (sv_numeq(), the non-flags APIs) should overload based on the current state, which is the current behaviour Setting Setting I plan to add a flags variant of sv_2num() (which isn't API /cry) and a flag for amagic_call() to force overload and ignore |
leonerd
left a comment
leonerd
left a comment
There was a problem hiding this comment.
Overall this seems a good direction. A couple of small comments.
|
|
||
| #define AMGf_want_list 0x0040 | ||
| #define AMGf_numarg 0x0080 | ||
| #define AMGf_force_scalar 0x0100 |
There was a problem hiding this comment.
This style is fine, but I think in general I usually add these in the form (1<<0), (1<<1), etc to make it clearer they are single bitflags, and easier to see gaps or where to add the next.
There was a problem hiding this comment.
I was following the existing formatting of that code.
Though my preference for flags is typically hex constants.
The current gap here is due to 0872de4 which removed a constant.
tonycoz
force-pushed
the
23918-cmp-api
branch
from
096a2ec to
e4ff664
Compare
some refactoring next, since sv_numeq_flags and sv_numne_flags are similar. Used a separate test file since putting every sv_num*() variant in the one file would be ugly Addresses GH Perl#23918 but isn't a direct fix
If nothing else putting them together may avoid someone doing `!sv_numeq(...)`
do_ncmp() expects simple SVs and for overloaded SVs will just compare the SvNV() of each SV, mishandling the case where the 0+ overload returns a large UV or IV that isn't exactly representable as an NV. # Conflicts: # ext/XS-APItest/t/sv_numeq.t # ext/XS-APItest/t/sv_numne.t # sv.c
These are all needed because overloading may make them inconsistent with <=> overloading.
similar to try_amagic_bin()
Discovered while working on another module, in many amagic_call() will use the current context when calling the overload sub, but these APIs might be called in XS code that simply needs a comparison, regardless of the current OP context.
and use it from the numeric comparison APIs.
instead of a special case in amagic_call()
and add AMGf_force_overload to amagic_call() which does the actual work.
modified the sv.c documentation since the perldelta sv_numeq link had multiple targets.
tonycoz
force-pushed
the
23918-cmp-api
branch
from
6c73ea0 to
178011c
Compare
2 participants
As discussed in #23918 this adds APIs corresponding to the other perl numeric comparison operators.
Separate APIs for each comparison a provided as each calls the appropriate overload for that comparison.
This also fixes a bug in sv_numeq() which didn't correctly handle the case where the operator itself isn't overloaded, but conversion to a number is, and that conversion returns a large integer not representable as a NV.