| msg171574 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-09-29 16:04 |
| "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. |
|
|
| msg171577 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2012-09-29 16:58 |
| Victor: you missed one. :-) #define Py_TPFLAGS_HAVE_STACKLESS_EXTENSION (3L<<15) |
|
|
| msg171578 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2012-09-29 17:14 |
| 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 |
|
|
| msg171579 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2012-09-29 17:25 |
| 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? |
|
|
| msg171655 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-09-30 18:28 |
| 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 |
|
|
| msg171720 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2012-10-01 14:42 |
| 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. |
|
|
| msg171723 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-10-01 14:51 |
| 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> > _______________________________________ > |
|
|
| msg171735 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2012-10-01 17:16 |
| > tp_flags type is long, not int. Indeed, and PyType_Spec.flags is int, not long. |
|
|
| msg171763 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-10-01 23:43 |
| > 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?) |
|
|
| msg172788 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-10-12 22:15 |
| Can anyone review my new patch, unsigned_tp_flags-2.patch? |
|
|
| msg174235 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-10-30 22:42 |
| 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 |
|
|
| msg174240 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-30 23:28 |
| Note that Py_TPFLAGS_INT_SUBCLASS already not used in Python 3.x. One bit of tp_flags can be freed. |
|
|
| msg174241 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2012-10-30 23:30 |
| > 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. |
|
|
| msg174242 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-10-30 23:31 |
| See also . |
|
|
| msg330670 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-29 09:11 |
| 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 |
|
|
| msg330678 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-29 11:07 |
| 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 |
|
|
| msg330679 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-29 11:07 |
| 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 |
|
|