python / cpython

The Python programming language
https://www.python.org
Other
63.17k stars 30.25k forks source link

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

Open skirpichev opened 10 months ago

skirpichev commented 10 months ago

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

// transformed to a separate issue per @serhiy-storchaka suggestion in https://github.com/python/cpython/pull/112145

Linked PRs

serhiy-storchaka commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

skirpichev commented 5 months ago

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.