Skip to content

Conversation

@chris-eibl
Copy link
Member

@chris-eiblchris-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.

void_Pypegen_stack_overflow(Parser*p);

Py_LOCAL_INLINE(void*)
staticinlinevoid*
Copy link
MemberAuthor

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 */
# definePy_LOCAL(type) static type __fastcall
# definePy_LOCAL_INLINE(type) static __inline type __fastcall
#else
# definePy_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
MemberAuthor

I think this is a skip news?

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

@vstinnervstinner 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

@pablogsalpablogsal 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

@pablogsalpablogsal merged commit e94d168 into python:mainApr 15, 2025
49 checks passed
@chris-eiblchris-eibl deleted the fix_clangcl_pegen branch April 15, 2025 16:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@chris-eibl@vstinner@pablogsal@picnixz