"1L << 31" is undefined if long is 32 bits long. We should use unsigned long for PyTypeObject.tp_flags.

Attached patch implements this fix. I don't know if it breaks somehow the backward compatibility.

See also issues #1621 and #7406.

Victor:  you missed one. :-)

#define Py_TPFLAGS_HAVE_STACKLESS_EXTENSION (3L<<15)

Actually, I think it'll be messy to make this work:  PyType_GetFlags is part of the stable ABI, so that's got to continue to return a long rather than an unsigned long.  And then we've got, in object.h:

#ifdef Py_LIMITED_API
#define PyType_HasFeature(t,f)  ((PyType_GetFlags(t) & (f)) != 0)
#else
#define PyType_HasFeature(t,f)  (((t)->tp_flags & (f)) != 0)
#endif

So we'd need an extra cast from long to unsigned long in the first branch.

I suggest instead just replacing that one occurrence of (1L << 31) with (long)(1UL << 31) to get around the undefined behaviour.


P.S. The docs would also need to be updated in these two files:

   Doc/c-api/typeobj.rst
   Doc/includes/typestruct.h

Martin, some things I'm not clear on w.r.t. the stable ABI.

(1) Do defined constants like Py_TPFLAGS_INT_SUBCLASS form part of the stable ABI?

(2) Would changing the type of Py_TPFLAGS_INT_SUBCLASS from long to unsigned long break the stable ABI?

Ok, I'm not Martin, but I'll try to answer. An ABI pertains to the interfaces exposed by compiled object code, not source code. With C, function signatures in compiled code don't keep any type information (*), and they obviously doesn't know about macros either. So I'd answer "No" to both questions :-)

(*) it would be different with C++, because of name mangling

What matters is that precompiled stay compatible; in addition, existing source code should continue to compile unmodified.

In the specific case, the flags type also shows up in PyType_Spec. As a consequence, the actual TPFLAGS_ values *do* constitute a part of the API.

OTOH, a number of the flags are not considered part of the API at all (unfortunately, they aren't explicitly excluded, either). Before we make such a change, we should really declare what flags are meant to be by an extension module, and what flags are implementation details only to be used by the object runtime itself.

Wrt. the proposed change: changing tp_flags to unsigned int is fine. I cannot see any real problem with changing PyType_Spec.flags to unsigned int - changing it to unsigned long would be incompatible on some systems.

Wrt. changing the existing flags: I'd prefer some deprecation procedure that just bans them from being used in an extension module (ultimately wrapping them within Py_BUILD_CORE). Once they are deprecated, changing their type is clearly fine.

tp_flags type is long, not int.
Le 1 oct. 2012 16:42, "Martin v. Löwis" <report@bugs.python.org> a écrit :

>
> Martin v. Löwis added the comment:
>
> What matters is that precompiled stay compatible; in addition, existing
> source code should continue to compile unmodified.
>
> In the specific case, the flags type also shows up in PyType_Spec. As a
> consequence, the actual TPFLAGS_ values *do* constitute a part of the API.
>
> OTOH, a number of the flags are not considered part of the API at all
> (unfortunately, they aren't explicitly excluded, either). Before we make
> such a change, we should really declare what flags are meant to be by an
> extension module, and what flags are implementation details only to be used
> by the object runtime itself.
>
> Wrt. the proposed change: changing tp_flags to unsigned int is fine. I
> cannot see any real problem with changing PyType_Spec.flags to unsigned int
> - changing it to unsigned long would be incompatible on some systems.
>
> Wrt. changing the existing flags: I'd prefer some deprecation procedure
> that just bans them from being used in an extension module (ultimately
> wrapping them within Py_BUILD_CORE). Once they are deprecated, changing
> their type is clearly fine.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue16086>
> _______________________________________
>

> tp_flags type is long, not int.

Indeed, and PyType_Spec.flags is int, not long.

> tp_flags type is long, not int.

Oh, I misunderstood what MvL wrote, sorry. I missed PyType_Spec structure.

Here is an updated and more complete patch. I changed the return code of PyType_GetFlags(), instead of changing PyType_HasFeature() macro.

> OTOH, a number of the flags are not considered part of the API at all (unfortunately, they aren't explicitly excluded, either). Before we make such a change, we should really declare what flags are meant to be by an extension module, and what flags are implementation details only to be used by the object runtime itself.

Can't we decide that later? (in other issue?)

Can anyone review my new patch, unsigned_tp_flags-2.patch?

New changeset 10795aac6df7 by Victor Stinner in branch 'default':
Issue #16086: PyTypeObject.tp_flags and PyType_Spec.flags are now unsigned
http://hg.python.org/cpython/rev/10795aac6df7

Note that Py_TPFLAGS_INT_SUBCLASS already not used in Python 3.x. One bit of tp_flags can be freed.

> Note that Py_TPFLAGS_INT_SUBCLASS already not used in Python 3.x. One bit of tp_flags can be freed.

Please open a new issue if you consider this issue important enough.

See also issue9307.

New changeset 9fbcfc08e5814d7aa9287740187e461425a99f67 by Victor Stinner (Eddie Elizondo) in branch 'master':
bpo-16086: Fix PyType_GetFlags() documentation (GH-10758)
https://github.com/python/cpython/commit/9fbcfc08e5814d7aa9287740187e461425a99f67

New changeset e754159ef0af99a4124dd041ab7ceb77fcc922ad by Victor Stinner (Miss Islington (bot)) in branch '3.7':
bpo-16086: Fix PyType_GetFlags() documentation (GH-10758) (GH-10789)
https://github.com/python/cpython/commit/e754159ef0af99a4124dd041ab7ceb77fcc922ad

New changeset 2a852a2b122cf9545909234cdc8fca564dc8f805 by Victor Stinner (Miss Islington (bot)) in branch '3.6':
bpo-16086: Fix PyType_GetFlags() documentation (GH-10758) (GH-10790)
https://github.com/python/cpython/commit/2a852a2b122cf9545909234cdc8fca564dc8f805