Remove check for float subclasses with nb_float == NULL in PyNumber_Float() ?

[skirpichev]

Similar check for int subclasses without nb_int was removed in 31a6554 from PyNumber_Long().

// transformed to a separate issue per @serhiy-storchaka suggestion in #112145

Linked PRs

• gh-112636: remove check for float subclasses without nb_float #112637

[serhiy-storchaka]

I still not sure what is more correct. Should we remove the check for float subclasses without nb_float? Did I make a mistake when I deleted the check for int subclasses without nb_int? Both cases are pretty hypothetical now, but we can't change the code without reason, and I don't remember my reasoning.

cc @mdickinson

[skirpichev]

Simple question: when it could be NULL? Clearly, this won't work for simple static types with tp_base set to PyFloat_Type (or it's subtype). Multiple inheritance?

[serhiy-storchaka]

You can create a float subclass in C and explicitly set nb_float to NULL.

[skirpichev]

You can create a float subclass in C and explicitly set nb_float to NULL

@serhiy-storchaka, I think in this way we could break anything:)

Here is a minimal example, when nb_float == NULL condition could be triggered. (Let me know if you had in mind something simpler.) Should we support such scenario? I doubt.

Subclass code:

static PyNumberMethods xxx_as_number = {
    .nb_float = NULL,  /* nb_float will be inherited anyway */
};

static PyObject*
fun(PyObject *o, PyObject *Py_UNUSED(ignored))
{
    PyNumberMethods *m = Py_TYPE(o)->tp_as_number;
    m->nb_float = NULL;
    Py_RETURN_NONE;
}

static PyMethodDef xxx_methods[] = {
    {"break_it", (PyCFunction) fun, METH_NOARGS, NULL},
    {NULL},
};

PyTypeObject XXX_Type = {
    PyVarObject_HEAD_INIT(NULL, 0)
    .tp_name = "xxx",
    .tp_basicsize = sizeof(PyFloatObject),
    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
    .tp_base = &PyFloat_Type,
    .tp_as_number = &xxx_as_number,
    .tp_methods = xxx_methods,
};

Session example:

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

Edit: pr description was updated with this example.

[skirpichev]

@serhiy-storchaka, I'm closing issue as well. Let me know if we should add a test for this.

PS: As noted in the pr #112637 (comment), the integer workaround shouldn't be restored.