Repository navigation
Define guideline for parameter types: PyObject vs specific type #29
Description
See python/devguide#1127 issue.
See also capi-workgroup/problems#31 issue.
See python/devguide#1127 issue.
See also capi-workgroup/problems#31 issue.
Activity
encukou commentedon Oct 17, 2023
IMO, we should continue to use
PyObject*to keep the current API consistent.XXX: To me,
PyTypeObjectis the type that needs the most casting betweenPyObject*and the concrete type. Should we allowPyTypeObject*?In a future API layer I'd love to switch to concrete types, but that's for the
revolutionrepo: capi-workgroup/api-revolution#3zooba commentedon Oct 17, 2023
I think this is only because of static type definitions? If we'd had heap types from the start, we'd likely have made them
PyObject*and made the caller responsible for handling them. (I'm personally quite a fan of "you instantiate a type withPyObject_Call".)vstinner commentedon Oct 17, 2023
While I know that it's not shared opinion, but in the long term, I would prefer to only use one type for all object arguments:
PyObject*, and make this type opaque. No more PyDictObject, PyTypeObject, or anything else. Just PyObject.Issues with using other types was discussed in PR python/cpython#106005.
gvanrossum commentedon Oct 17, 2023
Since we're supposed to decide this in this tracker, let's use this issue to debate it instead of linking to other issues (we should considered the "problems" repo closed at this point -- we will soon be able to read all about it in PEP 733).
I agree that in the evolutionary development of the C API we should stick to
PyObject *as much as possible. I also agree that possiblyPyTypeObject *may have to be a historical exception.In the radical department we could possibly do this differently, but I personally would prefer not to -- however, I'll debate that when we get to it in capi-workgroup/api-revolution#3.
encukou commentedon Oct 18, 2023
No, it's also lots of functions like
PyType_HasFeature,PyType_IsSubtype,PyType_GetName,PyDescr_NewMethod, even the newPyType_GetDict.I'm fine sticking to
PyObject *in new evolutionary API, just to make things consistent.What does this mean for type checking? Should API
assertthat the correct type was passed in? Or should it raise a runtimeTypeError? (That would propably mean performance-sensitive code will grow internal API that skips the check -- that is what capi-workgroup/api-revolution#3 aims to avoid.)vstinner commentedon Oct 18, 2023
Here I'm talking about the hypothetical case of "opaque types" just to explain my point, but I'm not really arguing to move towards that or not here. It's just to consider all cases.
On the API side, we can keep
PyTypeObject*type even if everything become opaque outside Python. We can keep all types, maybe just make all aliases to a single unique opaque PyObject type. Liketypedef PyObject PyTypeObject;. This is not exactly an alias but a different type, no? I'm not sure.In C, it's perfectly fine to define a type with an undefined structure:
typedef struct PyTypeObject PyTypeObject;without having to declarestruct PyTypeObject(members).My argument here is more the desire for the long term to remove members from structures, but keep type names. Example: using
PyTypeObjectis fine, but in the long term,type->tp_nameshould become a compiler error.Well, making
PyTypeObjectopaque will be a long journey, I started to take notes: C API: Investigate how the PyTypeObject members can be removed from the public C API. The good part is that PyTypeObject members are already excluded from the limited C API!If we change
PyAPI_FUNC(PyObject *) PyType_GetQualName(PyTypeObject *);declaration withPyAPI_FUNC(PyObject *) PyType_GetQualName(PyObject *);: change parameter type toPyObject*, it will introduce compiler warnings. I'm not sure of the value of such change. Maybe tomorrow, PyTypeObject will just be an alias to an opaque PyObject type.So I think that we should consider multiple things:
The consistency concern was discussed when adding a new PyDict_GetItemRef() API. Mark Shannon asked to use
PyDictObject*type, whereas others preferred to keepPyObject*. At the end, the consistency was chosen: consistency with other PyDict functions which takePyObject*and notPyDictObject*. Example:PyAPI_FUNC(PyObject *) PyDict_GetItem(PyObject *mp, PyObject *key);.vstinner commentedon Oct 18, 2023
Another data point: PEP 670 – Convert macros to functions in the Python C API was approved with Avoid the cast in the limited C API version 3.11:
It means that APIs which take
PyObject*such as Py_INCREF() no longer accept types other thanPyObject*and the caller is responsible to do the cast.As @encukou wrote, I think that in practice,
PyTypeObject*is the most common type different thanPyObject*which is widely used in C extensions. As @zooba noted, there are already inconsistencies when usingPyTypeObject*: PyObject_Call() can be used to create a type, but it returnsPyObject*. Then PyType functions expect aPyTypeObject*parameter. But many Python C API expect a PyObject again, such as Py_INCREF().In the non-limited API, there are multiple APIs which accept "any type" (such as
PyTypeObject*) and then transform the result to aPyObject*. Example:Py_NewRef().Example from Python code base:
Input:
PyTypeObject*. Py_NewRef() output:PyObject*which requires a cast toPyTypeObject*.Between you and me, I would prefer to use
PyObject*everywhere. But I'm not sure if it makes sense, nor how many compiler warnings we would have to fight to migrate to that :-(encukou commentedon Oct 18, 2023
I opened #27 for opaque structs (and also the structs that aren't opaque).
Let's keep this issue about
PyObject*vs. concrete types?And yes, guidelines should be about new API. You're right that we can't really change existing API.
zooba commentedon Oct 18, 2023
I meant that those functions used
PyTypeObject*because it was more convenient with already having defined types as a static struct PyTypeObject.1 People didn't have to write(PyObject*)my_typeas often. But we "know" that they're the same shape underneath2 so the cast isn't really changing anything.If we'd used the time machine on this one, they probably would've been
PyObject*from the start, and we'd do the cast everywhere. Unless Guido says that it was a deliberate design choice to rely on C's famously rich and reliable type system 🙃 But it doesn't seem like that:I also agree. All our deliberately public APIs should be
PyObject *to minimise abstraction leakage, and do their type checks (raising TypeErrors, not assertions). Fast APIs that skip checks should be the special case, not the default.Footnotes
Or were made to be consistent with existing API that was designed pre-heap types. ↩
Within the definition of how a
PyObjectis shaped. ↩encukou commentedon Oct 18, 2023
Sounds good. New API should use
PyObject*and do run-time type checking.We have #4 for the necessary exceptions.