gh-112636: remove check for float subclasses without nb_float

[skirpichev]

This condition could be triggered only in very special scenario, if users code explicitly override inherited nb_float value and set it to NULL:

/* nb_float will be inherited anyway if it's set to NULL here, so this is essentially redundant. */staticPyNumberMethodsxxx_as_number= {
.nb_float=NULL,
};
staticPyObject*break_it(PyObject*o, PyObject*Py_UNUSED(ignored))
{
Py_TYPE(o)->tp_as_number->nb_float=NULL;
Py_RETURN_NONE;
}
staticPyMethodDefxxx_methods[] = {
{"break_it", (PyCFunction) break_it, METH_NOARGS, NULL},
{NULL},
};
PyTypeObjectXXX_Type= {
PyVarObject_HEAD_INIT(NULL, 0)
.tp_name="xxx",
.tp_base=&PyFloat_Type,
.tp_as_number=&xxx_as_number,
.tp_methods=xxx_methods,
};

diff --git a/Objects/abstract.c b/Objects/abstract.c
index 7cca81464c..41216eeb9b 100644
--- a/Objects/abstract.c+++ b/Objects/abstract.c@@ -1640,6 +1640,7 @@ PyNumber_Float(PyObject *o)
/* A float subclass with nb_float == NULL */
if (PyFloat_Check(o)) {
+ printf("XXX\n");
return PyFloat_FromDouble(PyFloat_AS_DOUBLE(o));
}
return PyFloat_FromString(o);

>>> a = xxx(3.14)
>>> float(a) # nb_float == NULL is not triggered, due to inheritance
3.14
>>> a.break_it()
>>> float(a) # and only NOW this branch will work...
XXX
3.14

---

• Issue: Remove check for float subclasses with nb_float == NULL in PyNumber_Float() ? #112636

Second look.

[skirpichev]

CC @vstinner, does it make sense for you?

[vstinner]

>>> 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?

[skirpichev]

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.

[vstinner]

What is the behavior on this nb_float=NULL type with your change? Does return PyFloat_FromString(o) fail?

[skirpichev]

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.

[vstinner]

This change is a backward incompatible change. You should document it in a NEWS entry.

[serhiy-storchaka]

Sorry, but I now think that we should rather return a fallback for integers.

[skirpichev]

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 31a6554 (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?

[serhiy-storchaka]

Because PyFloat_AsDouble() never fails for instances of float subclasses. It does not use nb_float in that case.

[skirpichev]

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.

[skirpichev]

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:

cpython/Objects/abstract.c

Lines 1403 to 1405 in 31516c9

	if (PyLong_Check(item)) {
	returnPy_NewRef(item);
	}

and

cpython/Objects/abstract.c

Lines 1446 to 1448 in 31516c9

	if (result!=NULL&& !PyLong_CheckExact(result)) {
	Py_SETREF(result, _PyLong_Copy((PyLongObject*)result));
	}

I still think we shouldn't keep workarounds for broken subtypes. But if so, at least the nb_int workaround shouldn't be restored.

[skirpichev]

Ok, I'm closing this.