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

Activity

1. skirpichev

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Dec 3, 2023
2. skirpichev

   added a commit that references this issue on Dec 3, 2023

   pythongh-112636: remove check for float subclasses without nb_float

   56d7b83
3. serhiy-storchaka commented on Mar 22, 2024

   

   serhiy-storchaka

   on Mar 22, 2024

   Member

   More actions

   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
4. skirpichev commented on Mar 23, 2024

   

   skirpichev

   on Mar 23, 2024

   MemberAuthor

   More actions

   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?
5. serhiy-storchaka commented on Mar 26, 2024

   

   serhiy-storchaka

   on Mar 26, 2024

   Member

   More actions

   You can create a float subclass in C and explicitly set nb_float to NULL.
6. skirpichev commented on May 14, 2024

   

   skirpichev

   on May 14, 2024

   Last edited by skirpichev

   MemberAuthor

   More actions

   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.
7. skirpichev commented on Nov 2, 2024

   

   skirpichev

   on Nov 2, 2024

   MemberAuthor

   More actions

   @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.
8. skirpichev

   closed this as not plannedon Nov 2, 2024