Skip to content

GH-131296: fix clang-cl warning on Windows in sre.c #131593

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
merged 2 commits into from
Apr 1, 2025
Merged

GH-131296: fix clang-cl warning on Windows in sre.c #131593

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
547b5c4
fix clangcl warning in sre.c
chris-eibl Mar 22, 2025
853c1ff
just use "#if defined(_MSC_VER) && !defined(__clang__)"
chris-eibl Apr 1, 2025
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
6 changes: 5 additions & 1 deletion Modules/_sre/sre.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ static unsigned int sre_toupper(unsigned int ch) {
/* -------------------------------------------------------------------- */

#if defined(_MSC_VER)
#pragma optimize("agtw", on) /* doesn't seem to make much difference... */
# if defined(__clang__)
# pragma optimize("", on)
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix warning : unexpected argument 'agtw' to '#pragma optimize'; expected "" [-Wignored-pragmas]

# else
# pragma optimize("gt", on) /* doesn't seem to make much difference... */
Copy link
Member Author

Choose a reason for hiding this comment

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

agtw has been used in PY_LOCAL_AGGRESSIVE until #87332. Quote from @gvanrossum #87332 (comment)

A coworker happened to look at our use of #pragma optimize() for Windows (VS 2017) and noticed:

unless there’s something I’m missing, the ‘a’ and ‘w’ portions of the string being passed to the optimize pragma are not doing anything; I’m pretty sure they’ve been useless for 10+ years. The ‘g’ means turn on global optimizations and the ‘t’ means optimize for “time” (i.e. speed). See the documentation at https://docs.microsoft.com/en-us/cpp/preprocessor/optimize?view=msvc-160

It was then changed to gt (https://github.com/python/cpython/pull/24485/files) and entirely removed in https://github.com/python/cpython/pull/32023/files.

Copy link
Member Author

Choose a reason for hiding this comment

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

@zooba Again a very Windows (MSVC) specific one. Back then, you've changed PY_LOCAL_AGGRESSIVE from agtw to gt - agtw is really dusty.

OTOH, even though the comment says /* doesn't seem to make much difference... */ - I think it does, because this is also done for debug builds. But since /Ob1 is now used for debug builds (AFAIR by Victor), maybe this is not needed anymore at all?

# endif
#pragma warning(disable: 4710) /* who cares if functions are not inlined ;-) */
/* fastest possible local call under MSVC */
#define LOCAL(type) static __inline type __fastcall
Expand Down
Loading