FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-131288: use _AddressOfReturnAddress in _Py_get_machine_stack_pointer to silence MSVC warning in pycore_ceval.h for release builds by chris-eibl · Pull Request #131289 · python/cpython · GitHub

/ cpython Public

GH-131288: use _AddressOfReturnAddress in _Py_get_machine_stack_pointer to silence MSVC warning in pycore_ceval.h for release builds - #131289

Merged
colesbury merged 3 commits into
python:mainfrom
chris-eibl:fix_warning_pycore_ceval
Apr 4, 2025
ConversationCommits (3)ChecksFiles changed
Merged

GH-131288: use _AddressOfReturnAddress in _Py_get_machine_stack_pointer to silence MSVC warning in pycore_ceval.h for release builds#131289
colesbury merged 3 commits into
python:mainfrom
chris-eibl:fix_warning_pycore_ceval

Conversation

chris-eibl commented Mar 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

For details please see #131288.

I think this is a skip news.

bedevere-app Bot mentioned this pull request Mar 15, 2025
Closed
colesbury reviewed Mar 15, 2025
View reviewed changes
Comment thread Include/internal/pycore_ceval.h Outdated
#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable:4172)
#endif

colesbury Mar 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Use _AddressOfReturnAddress and avoid the UB on Windows?

https://learn.microsoft.com/en-us/cpp/intrinsics/addressofreturnaddress?view=msvc-170

chris-eibl Mar 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Great - even better!

See https://godbolt.org/z/7c9Gq9jzM, that should work out?

colesbury Mar 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah, it works on Windows arm64 too.

colesbury commented Mar 15, 2025

Copy link
Copy Markdown
Contributor

GitHub actions was having some trouble, so I merged main into the PR to give it a kick

colesbury commented Mar 15, 2025

Copy link
Copy Markdown
Contributor

!buildbot Windows

bedevere-bot commented Mar 15, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 66f5bc5 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131289%2Fmerge

The command will test the builders whose names match following regular expression: Windows

The builders matched are:

  • AMD64 Windows11 Refleaks PR
  • ARM64 Windows PR
  • AMD64 Windows PGO NoGIL PR
  • AMD64 Windows PGO PR
  • ARM64 Windows Non-Debug PR
  • AMD64 Windows10 PR
  • AMD64 Windows11 Non-Debug PR
  • AMD64 Windows11 Bigmem PR
  • AMD64 Windows Server 2022 NoGIL PR

chris-eibl commented Mar 15, 2025

Copy link
Copy Markdown
Member Author

Oh, my beloved test_int in https://buildbot.python.org/#/builders/1623/builds/13. Luckily re-run passed.

See #114911.

colesbury approved these changes Mar 17, 2025
View reviewed changes

colesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

colesbury requested a review from markshannon March 17, 2025 14:17

colesbury commented Mar 17, 2025

Copy link
Copy Markdown
Contributor

@markshannon - would you like to review this?

chris-eibl changed the title GH-131288: silence warning in pycore_ceval.h GH-131288: use _AddressOfReturnAddress in _Py_get_machine_stack_pointer to silence MSVC warning in pycore_ceval.h for release builds Mar 28, 2025

chris-eibl commented Apr 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Shall we get this into the last alpha? Or before beta 1? Or no need at all?

The AMD64 Windows PGO NoGIL failure is definitely unrelated, if this is blocking ...

colesbury commented Apr 4, 2025

Copy link
Copy Markdown
Contributor

Thanks for the reminder

colesbury merged commit 2009871 into python:main Apr 4, 2025
chris-eibl deleted the fix_warning_pycore_ceval branch April 4, 2025 13:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

colesbury colesbury approved these changes
markshannon Awaiting requested review from markshannon

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL