Skip to content

Conversation

@tobiasleibner
Copy link
Contributor

@tobiasleibner tobiasleibner commented Aug 25, 2025

Description

I am getting compiler warnings using clang-cl on Windows. These warnings are suppressed in pybind11 using PYBIND11_WARNING_DISABLE_CLANG which is defined as

#ifdef PYBIND11_COMPILER_CLANG
#    define PYBIND11_WARNING_DISABLE_CLANG(name) PYBIND11_PRAGMA(clang diagnostic ignored name)
#else
#    define PYBIND11_WARNING_DISABLE_CLANG(name)
#endif

However, for clang-cl, PYBIND11_COMPILER_CLANG is undefined because clang-cl also defines _MSC_VER and thus the compiler is detected as msvc.

Suggested changelog entry:

  • Fix compiler detection in pybind11/detail/pybind11_namespace_macros.h for clang-cl on Windows, to fix warning suppression macros.

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

From the PR description:

but that macro is defined to nothing

Is that correct? I'm thinking it must be using the MSVC definitions? Could you please review the PR description for correctness and also make the suggested changelog more specific (mention warning suppression pragmas)?

// only use compiler specifics if you need to check specific versions, e.g. Apple Clang vs. vanilla
// Clang.
#if defined(_MSC_VER)
#if defined(_MSC_VER) && !defined(__clang__) // clang-cl also defines _MSC_VER
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please try to move this block last (below __GNUC__), with a small but carefully written comment to explain why it should stay there? (Your favorite LLM can probably help doing this quickly.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done.

@tobiasleibner
Copy link
Contributor Author

tobiasleibner commented Aug 26, 2025

Is that correct? I'm thinking it must be using the MSVC definitions? Could you please review the PR description for correctness and also make the suggested changelog more specific (mention warning suppression pragmas)?

Yes, the definition is

#ifdef PYBIND11_COMPILER_CLANG
#    define PYBIND11_WARNING_DISABLE_CLANG(name) PYBIND11_PRAGMA(clang diagnostic ignored name)
#else
#    define PYBIND11_WARNING_DISABLE_CLANG(name)
#endif

I updated the PR description to include more details.

@rwgk
Copy link
Collaborator

rwgk commented Aug 27, 2025

Thanks!

@rwgk rwgk merged commit 3c0ee89 into pybind:master Aug 27, 2025
149 of 150 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants