python
gh-112636: remove check for float subclasses without nb_float
@serhiy-storchaka, could you please comment what's wrong with the pr on your opinion? Apparently, if removed block should be kept - then we must restore similar block in PyNumber_Long(), removed in 31a655411.
>>> a.break_it()
>>> float(a) # and only NOW this branch will work...
XXX
3.14
I don't understand the example. float(a) does now return XXX? Or does it print XXX? Which part of the code prints XXX?
I don't understand the example. float(a) does now return XXX?
Ah, that was just a debug print from the removed (by this pr) code. I've added a diff to description.
The point is that to trigger that code - you must explicitly set nb_float field in the derived class to NULL. But in this way we can break everything! Can't you set nb_add to NULL in a float subclass? Easy! But CPython has enough code, which assuming that float subclasses have working arithmetic dunder methods.
What is the behavior on this nb_float=NULL type with your change? Does return PyFloat_FromString(o) fail?
What is the behavior on this nb_float=NULL type with your change?
It fails with TypeError (after you run first break_it()).
Does return PyFloat_FromString(o) fail?
Yes, because o is not a subclass of str/bytes/etc.
Sorry, but I now think that we should rather return a fallback for integers.
This change is a backward incompatible change. You should document it in a NEWS entry.
Ok, I did. News entry was missed just as in 31a655411 (dropped similar check in PyNumber_Long).
I now think that we should rather return a fallback for integers.
But why?! Should we check on same ground that an int subtype has nb_add field set?
Because PyFloat_AsDouble() never fails for instances of float subclasses. It does not use nb_float in that case.
In same way we can recover in case when float subtype sets e.g. nb_negative to NULL. I think such subtypes are broken and it's better to fail quickly.
now think that we should rather return a fallback for integers.
There is a difference, if nb_int is set to NULL, but nb_index is not broken (at least not NULL) - then we got an equivalent fallback with _PyLong_Copy, see:
https://github.com/python/cpython/blob/31516c98dd7097047ba10da8dcf728c3d580f3d6/Objects/abstract.c#L1403-L1405
and
https://github.com/python/cpython/blob/31516c98dd7097047ba10da8dcf728c3d580f3d6/Objects/abstract.c#L1446-L1448
I still think we shouldn't keep workarounds for broken subtypes. But if so, at least the nb_int workaround shouldn't be restored.