Skip to content

GH-131296: fix clang-cl warning on Windows in pegen.h #131584

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.

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 1 commit into from
Apr 15, 2025

Conversation

chris-eibl
Copy link
Member

@chris-eibl chris-eibl commented Mar 22, 2025

Fix warning : fastcall calling convention is not supported on variadic function [-Wignored-attributes].
This warning is seen 7 times in a row: https://github.com/python/cpython/actions/runs/14006628024/job/39221297660#step:4:239

For 32bit builds of pythoncore, we'd see this another 7 times (and in case of PGO we'd triple up to 21 warnings).

MSVC seems to silently ignore it https://devblogs.microsoft.com/oldnewthing/20131128-00/?p=2543.

@@ -170,7 +170,7 @@ void *_PyPegen_raise_error_known_location(Parser *p, PyObject *errtype,
void _Pypegen_set_syntax_error(Parser* p, Token* last_token);
void _Pypegen_stack_overflow(Parser *p);

Py_LOCAL_INLINE(void *)
static inline void *
Copy link
Member Author

Choose a reason for hiding this comment

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

cpython/Include/pyport.h

Lines 190 to 197 in 18249d9

#if defined(_MSC_VER)
/* ignore warnings if the compiler decides not to inline a function */
# pragma warning(disable: 4710)
/* fastest possible local call under MSVC */
# define Py_LOCAL(type) static type __fastcall
# define Py_LOCAL_INLINE(type) static __inline type __fastcall
#else
# define Py_LOCAL(type) static type

__fastcall is anyway only in effect for 32bit builds. The warning stems from the _freeze_module, which is by default compiled in 32bit mode (unless you override PreferredToolArchitecture). Let's simply convert to static inline.

@chris-eibl
Copy link
Member Author

I think this is a skip news?

@chris-eibl chris-eibl changed the title GH-131296: fix warning in pegen.h GH-131296: fix clang-cl warning on Windows in pegen.h Mar 23, 2025
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. I agree with @chris-eibl's rationale that __fastcall is useless on this function.

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM Thanks a lot for the fix @chris-eibl

@pablogsal pablogsal merged commit e94d168 into python:main Apr 15, 2025
49 checks passed
@chris-eibl chris-eibl deleted the fix_clangcl_pegen branch April 15, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants