Skip to content
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

Jump to bottom

bpo-39573: Convert Py_REFCNT and Py_SIZE to functions #20429

Merged
merged 2 commits into from
May 27, 2020
Merged

bpo-39573: Convert Py_REFCNT and Py_SIZE to functions #20429

merged 2 commits into from
May 27, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 26, 2020 •
edited by bedevere-bot
Loading

Convert Py_REFCNT() and Py_SIZE() macros to static inline functions.
They cannot be used as l-value anymore: use Py_SET_REFCNT() and
Py_SET_SIZE() to set an object reference count and size.

Replace &Py_SIZE(self) with &((PyVarObject*)self)->ob_size
in arraymodule.c.

This change is backward incompatible on purpose, to prepare the C API
for a fully opaque PyObject structure.

https://bugs.python.org/issue39573

@vstinner
bpo-39573: Convert Py_REFCNT and Py_SIZE to functions
de117db 
Convert Py_REFCNT() and Py_SIZE() macros to static inline functions.
They cannot be used as l-value anymore: use Py_SET_REFCNT() and
Py_SET_SIZE() to set an object reference count and size.

Replace &Py_SIZE(self) with &((PyVarObject*)self)->ob_size
in arraymodule.c.

This change is backward incompatible on purpose, to prepare the C API
for an opaque PyObject structure.
@vstinner vstinner mentioned this pull request May 26, 2020
Closed
corona10
corona10 reviewed May 26, 2020
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

Objects/tupleobject.c:85:21: error: expression is not assignable
        Py_SIZE(op) = size;
        ~~~~~~~~~~~ ^

corona10
corona10 requested changes May 26, 2020
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

I left comment ;)

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@vstinner
Copy link
Member Author

Objects/tupleobject.c:85:21: error: expression is not assignable

Oh, Py_TRACE_REFS! Thanks for testing for me. It's now fixed.

corona10
corona10 approved these changes May 26, 2020
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@vstinner
Copy link
Member Author

This PR breaks Cython. I proposed cython/cython#3639 to use Py_SET_SIZE(): PR for the Cython master branch (then I will have to backport it to the 0.29.x branch). Maybe we should wait for a new Cython release compatible with this PR, before merging this PR.

@vstinner vstinner mentioned this pull request May 26, 2020
Merged
@vstinner
Copy link
Member Author

This PR breaks Cython. I proposed cython/cython#3639 to use Py_SET_SIZE(): PR for the Cython master branch (then I will have to backport it to the 0.29.x branch).

My fix landed in 0.29.x and master branches of Cython, great!

I asked cython/cython#3639 (comment) if a Cython 0.29.20 version will be released with the fix.

In the meanwhile, I think that it's fine to break Cython for a few days/weeks since we are at the beginning of the Python 3.10 dev cycle. So I merge my PR to be able to move on other C API issues.

@vstinner vstinner merged commit fe2978b into python:master May 27, 2020
@vstinner vstinner deleted the py_refnct_func branch May 27, 2020 12:55
@njsmith njsmith mentioned this pull request May 28, 2020
Closed
tacaswell added a commit to tacaswell/numpy that referenced this pull request May 29, 2020
@tacaswell
MNT: support python 3.10
405c6de 
In python/cpython#20290 CPython changed
`Py_TYPE` from a macro to an inline function.  This requires a code
change to us `Py_SET_TYPE` instead when using `Py_TYPE()` as a lvalue
in c code.

In python/cpython#20429 CPython changed
`Py_SIZE` from a macro to an inline function.  This requires a code
change to us `Py_SET_SIZE` instead of using `Py_SIZE` as a lvalue in c
code.
@tacaswell tacaswell mentioned this pull request May 29, 2020
Closed
@i64 i64 mentioned this pull request May 29, 2020
Closed
charris pushed a commit to charris/numpy that referenced this pull request May 29, 2020
@tacaswell @charris
MNT: support python 3.10
0e58c3c 
In python/cpython#20290 CPython changed
`Py_TYPE` from a macro to an inline function.  This requires a code
change to us `Py_SET_TYPE` instead when using `Py_TYPE()` as a lvalue
in c code.

In python/cpython#20429 CPython changed
`Py_SIZE` from a macro to an inline function.  This requires a code
change to us `Py_SET_SIZE` instead of using `Py_SIZE` as a lvalue in c
code.
cielavenir pushed a commit to cielavenir/numpy that referenced this pull request Aug 9, 2023
@tacaswell
MNT: support python 3.10
86e96a0 
In python/cpython#20290 CPython changed
`Py_TYPE` from a macro to an inline function.  This requires a code
change to us `Py_SET_TYPE` instead when using `Py_TYPE()` as a lvalue
in c code.

In python/cpython#20429 CPython changed
`Py_SIZE` from a macro to an inline function.  This requires a code
change to us `Py_SET_SIZE` instead of using `Py_SIZE` as a lvalue in c
code.
cielavenir pushed a commit to cielavenir/numpy that referenced this pull request Aug 22, 2023
@tacaswell
MNT: support python 3.10
3a863bf 
In python/cpython#20290 CPython changed
`Py_TYPE` from a macro to an inline function.  This requires a code
change to us `Py_SET_TYPE` instead when using `Py_TYPE()` as a lvalue
in c code.

In python/cpython#20429 CPython changed
`Py_SIZE` from a macro to an inline function.  This requires a code
change to us `Py_SET_SIZE` instead of using `Py_SIZE` as a lvalue in c
code.
@vstinner vstinner mentioned this pull request Mar 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 corona10 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vstinner @bedevere-bot @corona10 @the-knights-who-say-ni