-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: PYBIND11_PLATFORM_ABI_ID
Modernization Continued (platforms other than MSVC)
#5439
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
Changes from 20 commits
14f8425
e46982c
b72c42d
8369fdc
476c322
271720f
970a7eb
28081fc
fe2dbcb
9acf764
9fc9515
d412303
b6ccce3
a584398
23a5f2b
8fa10bf
02daf15
3f90808
b47be2d
e071edc
ca9e699
41daaa4
e34dc8b
75da5fb
f4cc9b9
fb38f03
09131c5
d05ea53
776d163
70639c1
7fb89ca
738b7ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,51 +12,32 @@ | |||||||||||||
#define PYBIND11_PLATFORM_ABI_ID_STRINGIFY(x) #x | ||||||||||||||
#define PYBIND11_PLATFORM_ABI_ID_TOSTRING(x) PYBIND11_PLATFORM_ABI_ID_STRINGIFY(x) | ||||||||||||||
|
||||||||||||||
// On MSVC, debug and release builds are not ABI-compatible! | ||||||||||||||
#if defined(_MSC_VER) && defined(_DEBUG) | ||||||||||||||
# define PYBIND11_BUILD_TYPE "_debug" | ||||||||||||||
#ifdef PYBIND11_COMPILER_TYPE | ||||||||||||||
// // To maintain backward compatibility (see PR #5439). | ||||||||||||||
# define PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE "" | ||||||||||||||
#else | ||||||||||||||
# define PYBIND11_BUILD_TYPE "" | ||||||||||||||
#endif | ||||||||||||||
|
||||||||||||||
// Let's assume that different compilers are ABI-incompatible. | ||||||||||||||
// A user can manually set this string if they know their | ||||||||||||||
// compiler is compatible. | ||||||||||||||
#ifndef PYBIND11_COMPILER_TYPE | ||||||||||||||
# if defined(_MSC_VER) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_msvc" | ||||||||||||||
# elif defined(__INTEL_COMPILER) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_icc" | ||||||||||||||
# elif defined(__clang__) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_clang" | ||||||||||||||
# elif defined(__PGI) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_pgi" | ||||||||||||||
# elif defined(__MINGW32__) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_mingw" | ||||||||||||||
# define PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE "_" | ||||||||||||||
# if defined(__MINGW32__) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "mingw" | ||||||||||||||
# elif defined(__CYGWIN__) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_gcc_cygwin" | ||||||||||||||
# elif defined(__GNUC__) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "_gcc" | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "gcc_cygwin" | ||||||||||||||
# elif defined(_MSC_VER) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "msvc" | ||||||||||||||
# elif defined(__INTEL_COMPILER) || defined(__clang__) || defined(__GNUC__) | ||||||||||||||
# define PYBIND11_COMPILER_TYPE "system" // Assumed compatible with system compiler. | ||||||||||||||
|
# elif defined(__INTEL_COMPILER) || defined(__clang__) || defined(__GNUC__) | |
# define PYBIND11_COMPILER_TYPE "system" // Assumed compatible with system compiler. | |
# elif defined(__GLIBC__) && (defined(__INTEL_COMPILER) || defined(__clang__) || defined(__GNUC__)) | |
# define PYBIND11_COMPILER_TYPE "glibc" // Assumed compatible with system compiler. | |
# elif defined(__APPLE__) && (defined(__INTEL_COMPILER) || defined(__clang__) || defined(__GNUC__)) | |
# define PYBIND11_COMPILER_TYPE "macos" // Assumed compatible with system compiler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this. Trying it out in the CI right now: ca9e699
The code is exactly as suggested, but I revised the comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. On second thought, this would break musl, android builds etc. Let's keep the earlier one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, gee: sorry I missed your last comment before.
I don't know about musl and android builds.
But I just revert ... simpler is better here, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of simplification, consider removing
defined(__INTEL_COMPILER)
The Intel classic compilers,
icc
for C andicpc
for C++, define__GNUC__
as well as__INTEL_COMPILER
.The last version was 2021.10 and it stopped shipping in 2023.
The "Intel(R) oneAPI DPC++/C++ Compilers",
icx
for C andicpx
for C++, do not define__INTEL_COMPILER
.They do define all of these:
__GNUC__
,__clang__
,__INTEL_CLANG_COMPILER
, and__INTEL_LLVM_COMPILER
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 738b7ec
Thanks @hpkfft for the suggestion!