Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-114828: parenthesize non-atomic macro definitions in pycore_symtable.h#115143

Merged
carljm merged 1 commit intopython:mainfrom
carljm:parmacros
Feb 7, 2024
Merged

gh-114828: parenthesize non-atomic macro definitions in pycore_symtable.h#115143
carljm merged 1 commit intopython:mainfrom
carljm:parmacros

Conversation

@carljm
Copy link
Member

@carljmcarljm commentedFeb 7, 2024
edited by bedevere-appbot
Loading

In#115139, I used the expression~DEF_FREE as a mask to clear theDEF_FREE bit in a symbol table entry.

Unfortunately,DEF_FREE is defined as#define DEF_FREE 2<<4, so this macro-expands to~2<<4, and the bitwise negation binds more tightly than the bit-shift, so this actually meant(~2)<<4 rather than the~(2<<4) which I intended.

This is why non-atomic macro bodies should always be parenthesized!

Fix it by parenthesizing all the non-atomic definitions inpycore_symtable.h, which not only fixes my~DEF_FREE, but also prevents anyone else making the same mistake in future.

Copy link
Member

@JelleZijlstraJelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks for figuring this out!

@JelleZijlstra
Copy link
Member

It might be worth backporting this to 3.11, though there will be some conflicts and it's unlikely that we'll make a lot of changes to the 3.11 symtable at this point. Up to you.

Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks!

@carljm
Copy link
MemberAuthor

Hmm. I think my (weak) inclination is not to backport to 3.11, given that AFAIK there were major symtable changes in 3.12 but not in 3.11, so it seems unlikely we'll be backporting many changes to 3.11, and even less likely that any such changes would be affected by this. This PR is actually a bugfix for 3.12 (given that my~DEF_FREE exists there), but for 3.11 it would be purely preventative. So I'd err on the side of being conservative with backports.

But like I say, this is a weak preference, and I'll happily backport to 3.11 if anyone feels strongly that that's the thing to do.

@carljmcarljm merged commit8f0998e intopython:mainFeb 7, 2024
@miss-islington-app
Copy link

Thanks@carljm for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@carljmcarljm deleted the parmacros branchFebruary 7, 2024 20:19
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestFeb 7, 2024
…symtable.h (pythonGH-115143)(cherry picked from commit8f0998e)Co-authored-by: Carl Meyer <carl@oddbird.net>
@bedevere-app
Copy link

GH-115149 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelFeb 7, 2024
carljm added a commit that referenced this pull requestFeb 7, 2024
…_symtable.h (GH-115143) (#115149)gh-114828: parenthesize non-atomic macro definitions in pycore_symtable.h (GH-115143)(cherry picked from commit8f0998e)Co-authored-by: Carl Meyer <carl@oddbird.net>
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull requestFeb 14, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@carljm@JelleZijlstra@AlexWaygood

Comments


[8]ページ先頭

©2009-2026 Movatter.jp