Skip to content

bpo-39573: Porting to Python 3.10: Py_SET_SIZE() macro #20610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vstinner merged 2 commits into from
Jun 4, 2020
Merged

bpo-39573: Porting to Python 3.10: Py_SET_SIZE() macro #20610

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
743f412
bpo-39573: Porting to Python 3.10: Py_SET_SIZE() macro
vstinner Jun 3, 2020
3feadc8
Rewrite macros
vstinner Jun 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions Doc/whatsnew/3.10.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,35 @@ Porting to Python 3.10

* Since :c:func:`Py_TYPE()` is changed to the inline static function,
``Py_TYPE(obj) = new_type`` must be replaced with ``Py_SET_TYPE(obj, new_type)``:
see :c:func:`Py_SET_TYPE()` (available since Python 3.9).
see :c:func:`Py_SET_TYPE()` (available since Python 3.9). For backward
compatibility, this macro can be used::

#if PY_VERSION_HEX < 0x030900A4
# define Py_SET_TYPE(obj, type) do { Py_TYPE(obj) = (type); } while (0)
#endif
Comment on lines +141 to +143
Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro doesn't behave the same way as the function in 3.9. I think a more accurate macro would be:

Suggested change
#if PY_VERSION_HEX < 0x030900A4
# define Py_SET_TYPE(obj, type) do { Py_TYPE(obj) = (type); } while (0)
#endif
#if PY_VERSION_HEX < 0x030900A4
# define Py_SET_TYPE(obj, type) ((Py_TYPE(obj)) = (type), (void)0)
#endif

which can be used as part of an expression like the function version of Py_SET_TYPE can:

return Py_SET_TYPE(my_obj, some_type), my_obj;

is legal in 3.9 and with my macro, but illegal with this PR.

Copy link
Copy Markdown
Member Author

@vstinner vstinner Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, the C language is so surprising sometimes :-) I designed Py_SET_SIZE() as a static inline function to ensure that the result is void and to prevent strange usage. But I didn't know that return Py_SET_TYPE(my_obj, some_type), (...); was legit.

I updated macros to use , (void)0. According to https://stackoverflow.com/a/25021889 this syntax is endorsed by the C standard, since NDEBUG uses #define assert(ignore) ((void)0).

Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding some precedent. I've seen "always use parens and do { } while(0);" given as macro advice forever, but I don't remember ever seeing this case before.

Copy link
Copy Markdown
Member Author

@vstinner vstinner Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meanwhile, I created the backport compatiblity header file :-) It defines a static inline function which returns void:
https://github.com/pythoncapi/pythoncapi_compat/blob/219cdd858595fb5d478117b0ef4758fedc3d2365/pythoncapi_compat.h#L29

Maybe tomorrow when it will be more mature (than 1 hour old), numpy might want to use it.


(Contributed by Dong-hee Na in :issue:`39573`.)

* Since :c:func:`Py_REFCNT()` is changed to the inline static function,
``Py_REFCNT(obj) = new_refcnt`` must be replaced with ``Py_SET_REFCNT(obj, new_refcnt)``:
see :c:func:`Py_SET_REFCNT()` (available since Python 3.9).
see :c:func:`Py_SET_REFCNT()` (available since Python 3.9). For backward
compatibility, this macro can be used::

#if PY_VERSION_HEX < 0x030900A4
# define Py_SET_REFCNT(obj, refcnt) do { Py_REFCNT(obj) = (refcnt); } while (0)
Copy link
Copy Markdown
Member Author

@vstinner vstinner Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tacaswell: In one of your PR, you wrote "typ" instead of "type". Was it a deliberate typo? Is there an issue in C++? Should I rename the variable to "new_type"?

Copy link
Copy Markdown
Member Author

@vstinner vstinner Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, my question is on the second parameter of Py_SET_TYPE() :-)

Copy link
Copy Markdown
Member Author

@vstinner vstinner Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built Py_SET_TYPE() macro with g++ and Python 3.8: I don't have any warning or error. "type" sounds like a valid parameter type.

Copy link
Copy Markdown
Contributor

@tacaswell tacaswell Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, that code come from @eric-wieser via numpy/numpy#16417 (comment)

#endif

(Contributed by Victor Stinner in :issue:`39573`.)

* Since :c:func:`Py_SIZE()` is changed to the inline static function,
``Py_SIZE(obj) = new_size`` must be replaced with ``Py_SET_SIZE(obj, new_size)``:
see :c:func:`Py_SET_SIZE()` (available since Python 3.9).
see :c:func:`Py_SET_SIZE()` (available since Python 3.9). For backward
compatibility, this macro can be used::

#if PY_VERSION_HEX < 0x030900A4
# define Py_SET_SIZE(obj, size) do { Py_SIZE(obj) = (size); } while (0)
#endif

(Contributed by Victor Stinner in :issue:`39573`.)

* Calling :c:func:`PyDict_GetItem` without :term:`GIL` held had been allowed
Expand Down