PyObject_HEAD_INIT is defined incorrectly

[Zuzu-Typ]

Bug report

Bug description:

Given that PyObject_HEAD_INIT is defined as:

#define PyObject_HEAD_INIT(type)    \
    {                               \
        { _Py_IMMORTAL_REFCNT },    \
        (type)                      \
    },

and _Py_IMMORTAL_REFCNT is defined as:

#define _Py_IMMORTAL_REFCNT _Py_CAST(Py_ssize_t, UINT_MAX)

the macro call is expanded to:

{ { (Py_ssize_t)(UINT_MAX) }, (type) }, 

First of all, this is different from the documentation, which reads:

This macro expands to:

_PyObject_EXTRA_INIT
1, type,

More importantly though, this introduces a compilation problem, when called with NULL (which is a common use case):

static PyTypeObject myType = {
	PyObject_HEAD_INIT(NULL)
	"my.type",
	sizeof(myTypeStruct),
// [...]	

the macro call PyObject_HEAD_INIT(NULL) expands to:

{ { (Py_ssize_t)(UINT_MAX) }, (NULL) }, 

Why is this important?
Let me walk you through what this does.

PyObject_HEAD_INIT is used to initialize the following struct:

typedef struct {
    PyObject ob_base;
    Py_ssize_t ob_size; /* Number of items in variable part */
} PyVarObject;

What happens is the following:

PyVarObject.ob_base = { (Py_ssize_t)(UINT_MAX) }
PyVarObject.ob_size = (NULL)

This is problematic. On compilers that interpret NULL as 0, this compiles successfully. On compilers that interpret NULL as nullptr, this causes a compile error. Either way, it does not do what was intended.

This issue was introduced with this commit: ea2c001#diff-87272721a5cf1cd9915d6f503f6a7bbefa2f26c935c7ce83ca78706afd0ad05aL81

Thanks to @Weissnix4711 for helping me trace this issue.

This issue introduced a compile error in my package PyGLM: Zuzu-Typ/PyGLM#236 (only for systems using a compiler that defines NULL as nullptr)

CPython versions tested on:

3.12

Operating systems tested on:

No response

[ZeroIntensity]

I think this is user-error. PyVarObject_HEAD_INIT should be used for types, not PyObject_HEAD_INIT.

Since, as you mentioned, a type object is a PyVarObject, the NULL (which is supposed to be a pointer to a type) gets passed as the size, which is why this is failing. I don't think this is specific to the immortal objects implementation, because I was unable to compile the reproducer on any Python version. (I suspect PyObject_HEAD_INIT for types is a remnant of Python 2 extensions, but someone will have to fact-check me on that.)

(@encukou, this needs pending.)

[Zuzu-Typ]

According to the documentation PyObject_HEAD_INIT corresponds to types that use PyObject_HEAD, which reads:

This is a macro used when declaring new types which represent objects without a varying length. The PyObject_HEAD macro expands to:
PyObject ob_base;

(the macro actually expands to:)

PyVarObject ob_base;

There also is PyObject_VAR_HEAD, which corresponds to PyVarObject_HEAD_INIT and reads:

This is a macro used when declaring new types which represent objects with a length that varies from instance to instance. The PyObject_VAR_HEAD macro expands to:
PyVarObject ob_base;

Since PyObject_HEAD is used for types that don't have a variable length, PyObject_HEAD_INIT does not require a size argument.
It makes no sense to initialize it with PyVarObject_HEAD_INIT. If PyObject_HEAD was defined as stated by the documentation, using PyVarObject_HEAD_INIT would actually break.

The documentation does not show how to use either of the macros.
In the documentation there is only a single example for how to define types, which uses PyObject_HEAD and PyVarObject_HEAD_INIT. That is logically incorrect and does not fit the written documentation.
Since this is the only example though I presume people simply accepted doing it like this, regardless of whether it's correct according to the documentation or not.

Prior to 3.12 this would compile on any machine, because PyObject_HEAD_INIT used to expand to:

{ 1, type },

At first glance you might be tempted to say that makes no difference, but C/C++ handles this differently:

PyVarObject.ob_type = { 1, type }
PyVarObject.ob_size = // 0 or not defined.I didn't check 

See the difference in Visual Studio (I used 1 instead of UINT_MAX for demonstration purposes):


For better readability:

// before python 3.12
PyVarObject {
     _object{
        (Py_ssize_t)1i64, 
        (PyTypeObject *)0
    }
} 

// since python 3.12
PyVarObject{ 
    _object{
        (Py_ssize_t)1i64
    }, 
    (Py_ssize_t)0i64
} 

So what happens is that in the end the expected target state is reached more or less by accident.

As you can see PyObject_HEAD and PyObject_HEAD_INIT are not defined in a way that they work together correctly. By chance, on most machines they do. The documentation is out of date and the example has been incorrect for years I presume.

This isn't a user error.

[serhiy-storchaka]

You should use PyVarObject_HEAD_INIT instead of PyObject_HEAD_INIT for initializing a PyTypeObject.

[ZeroIntensity]

I think this is up to a little misinterpretation from the docs (which is possibly worth making more clear):

Since PyObject_HEAD is used for types that don't have a variable length, PyObject_HEAD_INIT does not require a size argument.

That would be referring to the object instance structure, not the type object definition. For example:

typedef struct {
    PyObject_HEAD
    int foo;
    int bar;
} MyObject;

The above structure must have its ob_base initialized with PyObject_HEAD_INIT. Likewise, if it used PyObject_VAR_HEAD, then it should be initialized with PyVarObject_HEAD_INIT. As a user, I would assume mixing the two would break things.

A PyTypeObject is always a PyVarObject (see here), and needs to be initialized as such. I don't think we've ever allowed initializing a PyVarObject with PyObject_HEAD_INIT (as you said, it might work on some compilers by accident, but that's not the point). In my case, GCC doesn't compile a type object using PyObject_HEAD_INIT on all versions going back to 3.7.

[ZeroIntensity]

In my case, GCC doesn't compile a type object using PyObject_HEAD_INIT on all versions going back to 3.7.

Ah wait, my reproducer was wrong. I do see this working on older versions, but I wouldn't really call this a regression -- we never were supposed to support it in the first place.

[Zuzu-Typ]

That would be referring to the object instance structure, not the type object definition.

Alright, that does clear up things a bit. Since the documentation never clearly states where to use the macro and where not to (or how to initialize a PyTypeObject) I must have assumed that I need to use PyObject_HEAD_INIT for types (i.e. type objects) without varying length, which is incorrect.

For me that begs the question though, what is the purpose of PyObject_HEAD_INIT? You said

The above structure must have its ob_base initialized with PyObject_HEAD_INIT

Although true in a programmatical sense, if I'm not mistaken the instances must be created like this:

MyObject * my_object = (MyObject*) (MyObjectType->tp_alloc(MyObjectType, 0));

What would be the actual use-case for PyObject_HEAD_INIT?

[ZeroIntensity]

tp_alloc would only be used if you wanted the instance on the heap. PyObject_HEAD_INIT would be used if you wanted to make an immortal statically-allocated instance of an object (for example, True and False in Python both use PyObject_HEAD_INIT).

[Zuzu-Typ]

Thanks for letting me know.

Should I close the issue or do you want to use it to track possible documentation changes?

[ZeroIntensity]

It might be worth clarifying, but it also might just be documentation bloat. @serhiy-storchaka, what do you think? Should we explicitly note in the docs that PyVarObject_HEAD_INIT must be used to initialize type objects?

[encukou]

Where would that note go?
I don't see the place in the docs that would lead people to use PyObject_HEAD_INIT for types. But I probably suffer from expert blindness.

[ZeroIntensity]

It would probably go here. The misinterpretation in this case was that PyObject_HEAD_INIT mentions that it's used for types, rather than object structures. Though, PyVarObject_HEAD_INIT says the same thing.

I think it's fine to close this. IMO, adding extra clarification would just be more bloat for the docs.

[encukou]

Yeah, especially with static types getting de-emphasized, it doesn't seem like a good idea to single them out in PyObject_HEAD_INIT docs.