◐ Shell
reader mode source ↗
Skip to content

bpo-33608: Normalize atomic macros so that they all expect an atomic struct#12877

Merged
zooba merged 1 commit into
python:masterfrom
zooba:bpo-33608
Apr 22, 2019
Merged

bpo-33608: Normalize atomic macros so that they all expect an atomic struct#12877
zooba merged 1 commit into
python:masterfrom
zooba:bpo-33608

Conversation

@zooba

@zooba zooba commented Apr 18, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@ericsnowcurrently @paulmon This should be an alternative fix to #12665.

Basically, any macro/function starting with _Py_atomic now always requires a reference to a _Py_atomic struct, and never the extracted value.

https://bugs.python.org/issue33608

@zooba zooba requested a review from ericsnowcurrently April 18, 2019 16:01
@paulmon

paulmon commented Apr 18, 2019

Copy link
Copy Markdown
Contributor

Looks good. It makes sense to me that the macro name implies the argument type.
I have also confirmed that this builds for ARM and runs as expected

@zooba

zooba commented Apr 18, 2019

Copy link
Copy Markdown
Member Author

I triggered a custom buildbot run to validate this across the fleet.

@paulmon paulmon mentioned this pull request Apr 18, 2019
Closed
@zooba

zooba commented Apr 18, 2019

Copy link
Copy Markdown
Member Author

The custom buildbots seem happy enough, so I'll give Eric some time to review if he wants and then I'll merge.

ericsnowcurrently
ericsnowcurrently approved these changes Apr 18, 2019
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

The approach is good. It definitely reads better. :) I only had a chance to skim through the PR. If you'd like me to dig in and verify each line (which could be valuable given the sensitivity of this code) then I'll see about that tomorrow afternoon. Otherwise LGTM.

@zooba zooba merged commit 2644907 into python:master Apr 22, 2019
@zooba

zooba commented Apr 22, 2019

Copy link
Copy Markdown
Member Author

Mostly wanted an "OK" on the approach. The compiler is doing the most important checks

@zooba zooba deleted the bpo-33608 branch April 22, 2019 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@zooba @paulmon @ericsnowcurrently @the-knights-who-say-ni @bedevere-bot