Skip to content

gh-135927: Fix MSVC Clatest C builds - #135935

Merged
Fidget-Spinner merged 2 commits into
python:mainfrom
Fidget-Spinner:fix-latest-c
Jun 25, 2025
Merged

gh-135927: Fix MSVC Clatest C builds#135935
Fidget-Spinner merged 2 commits into
python:mainfrom
Fidget-Spinner:fix-latest-c

Conversation

@Fidget-Spinner

@Fidget-SpinnerFidget-Spinner commented Jun 25, 2025

Copy link
Copy Markdown
Member

The problem is that we are checking for the latest C assuming only GCC and Clang supports it, but in reality MSVC supports it (on some configurations) too.

The fix is to account for MSVC.

@Fidget-Spinner

Copy link
Copy Markdown
MemberAuthor

Verified locally that building with clang-cl also works.

@zooba

Copy link
Copy Markdown
Member

Change looks fine to me, though I haven't dug into why the availability differs. If this works, then I guess it's fine.

@Fidget-Spinner
Fidget-Spinner merged commit a88b49c into python:mainJun 25, 2025
40 checks passed
@Fidget-Spinner
Fidget-Spinner deleted the fix-latest-c branch June 25, 2025 15:02
@Fidget-Spinner

Copy link
Copy Markdown
MemberAuthor

Thanks Steve!

@chris-eibl

Copy link
Copy Markdown
Member

I am on Mobile, but LGTM

Comment threadInclude/pyport.h
// to prevent C++ compiler warnings. On C23 and newer and on C++11 and newer,
// _Py_NULL is defined as nullptr.
#if (defined (__STDC_VERSION__) && __STDC_VERSION__ > 201710L) \
#if (defined(__GNUC__) || defined(__clang__)) && \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since there are other C compilers in the wild, it would be better to check for !defined(_MSC_VER) instead of being specific about GCC and clang.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Hmm you're right. Do you want to open a PR for this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I created #135987 : can you test it with MSC with /std:clatest?

AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Fidget-Spinner@zooba@chris-eibl@vstinner