Skip to content
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

PYBIND11_PLATFORM_ABI_ID Modernization Continued (platforms other than MSVC) #5439

Merged
merged 32 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
14f8425
THIS IS JUST A START: First attempt to combine information from PR #4…
rwgk Nov 10, 2024
e46982c
Merge branch 'master' into pybind11_platform_abi_id_cont
rwgk Nov 18, 2024
b72c42d
Include GXX_ABI and USE_CXX in the identifier
cryos Nov 20, 2024
8369fdc
style: pre-commit fixes
pre-commit-ci[bot] Nov 20, 2024
476c322
Use `gxx_abi_1xxx` and simplify the Clang string
cryos Nov 21, 2024
271720f
Error if `_GLIBCXX_USE_CXX11_ABI` is not defined
cryos Nov 21, 2024
970a7eb
Merge branch 'master' into pybind11_platform_abi_id_cont
rwgk Nov 24, 2024
28081fc
Change `usecxx11` to `use_cxx11_abi` for correspondence with `_GLIBCX…
rwgk Nov 24, 2024
fe2dbcb
`PYBIND11_COMPILER_TYPE` overhaul, mainly: replace `_icc`, `_clang`, …
rwgk Nov 24, 2024
9acf764
Merge branch 'master' into pybind11_platform_abi_id_cont
rwgk Nov 25, 2024
9fc9515
Add NVHPC (__PGI) to the list of compilers compatible with system com…
rwgk Nov 25, 2024
d412303
Fix oversight: remove __NVCOMPILER elif branch in PYBIND11_BUILD_ABI …
rwgk Nov 25, 2024
b6ccce3
Revert "Fix oversight: remove __NVCOMPILER elif branch in PYBIND11_BU…
rwgk Nov 26, 2024
a584398
Revert "Add NVHPC (__PGI) to the list of compilers compatible with sy…
rwgk Nov 26, 2024
23a5f2b
Define NVHPC PYBIND11_BUILD_ABI using __GNUC__, __GNUC_MINOR__, _GLIB…
rwgk Nov 26, 2024
8fa10bf
Use _GLIBCXX_USE_CXX11_ABI to detect libstdc++, then assume that NVHP…
rwgk Nov 26, 2024
02daf15
Enhance NVHPC comment and limited future proofing.
rwgk Nov 26, 2024
3f90808
The `PYBIND11_STDLIB` is obsolete but kept around to maintain backwar…
rwgk Dec 1, 2024
b47be2d
Move `PYBIND11_BUILD_TYPE` down in the file, so that the order of mac…
rwgk Dec 1, 2024
e071edc
Introduce `PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE`:
rwgk Dec 2, 2024
ca9e699
Apply suggestion by @isuruf, with revised comments (code is as sugges…
rwgk Dec 3, 2024
41daaa4
Make determination of `PYBIND11_COMPILER_TYPE` `"macos"` or `"glibc"`…
rwgk Dec 3, 2024
e34dc8b
Add `PYBIND11_COMPILER_TYPE` `emscripten`
rwgk Dec 6, 2024
75da5fb
Add `PYBIND11_COMPILER_TYPE` `graalvm`
rwgk Dec 6, 2024
f4cc9b9
Merge branch 'master' into pybind11_platform_abi_id_cont
rwgk Dec 8, 2024
fb38f03
Revert "Add `PYBIND11_COMPILER_TYPE` `graalvm`"
rwgk Dec 12, 2024
09131c5
Revert "Add `PYBIND11_COMPILER_TYPE` `emscripten`"
rwgk Dec 12, 2024
d05ea53
Revert "Make determination of `PYBIND11_COMPILER_TYPE` `"macos"` or `…
rwgk Dec 12, 2024
776d163
Revert "Apply suggestion by @isuruf, with revised comments (code is a…
rwgk Dec 12, 2024
70639c1
Merge branch 'master' into pybind11_platform_abi_id_cont
rwgk Dec 12, 2024
7fb89ca
Merge branch 'master' into pybind11_platform_abi_id_cont
rwgk Dec 12, 2024
738b7ec
Remove `defined(__INTEL_COMPILER)` as suggested by @hpkfft under http…
rwgk Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 40 additions & 41 deletions include/pybind11/conduit/pybind11_platform_abi_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Copy link

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 and icpc 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 and icpx for C++, do not define __INTEL_COMPILER.
They do define all of these: __GNUC__, __clang__, __INTEL_CLANG_COMPILER, and __INTEL_LLVM_COMPILER.

Copy link
Collaborator Author

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!

# define PYBIND11_COMPILER_TYPE "system" // Assumed compatible with system compiler.
Copy link
Contributor

Choose a reason for hiding this comment

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

This still feels weird.

How about?

Suggested change
# 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.

Copy link
Collaborator Author

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.

Copy link
Contributor

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.

Copy link
Collaborator Author

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.

# else
# define PYBIND11_COMPILER_TYPE "_unknown"
# error "Unknown PYBIND11_COMPILER_TYPE: PLEASE REVISE THIS CODE."
# endif
#endif

// Also standard libs
// PR #5439 made this macro obsolete. However, there are many manipulations of this macro in the
// wild. Therefore, to maintain backward compatibility, it is kept around.
#ifndef PYBIND11_STDLIB
# if defined(_LIBCPP_VERSION)
# define PYBIND11_STDLIB "_libcpp"
# elif defined(__GLIBCXX__) || defined(__GLIBCPP__)
# define PYBIND11_STDLIB "_libstdcpp"
# else
# define PYBIND11_STDLIB ""
# endif
# define PYBIND11_STDLIB ""
#endif

#ifndef PYBIND11_BUILD_ABI
# if defined(__GXX_ABI_VERSION) // Linux/OSX.
# define PYBIND11_BUILD_ABI "_cxxabi" PYBIND11_PLATFORM_ABI_ID_TOSTRING(__GXX_ABI_VERSION)
# elif defined(_MSC_VER) // See PR #4953.
# if defined(_MSC_VER) // See PR #4953.
# if defined(_MT) && defined(_DLL) // Corresponding to CL command line options /MD or /MDd.
# if (_MSC_VER) / 100 == 19
# define PYBIND11_BUILD_ABI "_md_mscver19"
Expand All @@ -72,17 +53,35 @@
# error "Unknown major version for MSC_VER: PLEASE REVISE THIS CODE."
# endif
# endif
# elif defined(__NVCOMPILER) // NVHPC (PGI-based).
# define PYBIND11_BUILD_ABI "" // TODO: What should be here, to prevent UB?
# elif defined(_LIBCPP_ABI_VERSION) // https://libcxx.llvm.org/DesignDocs/ABIVersioning.html
# define PYBIND11_BUILD_ABI \
"_libcpp_abi" PYBIND11_PLATFORM_ABI_ID_TOSTRING(_LIBCPP_ABI_VERSION)
# elif defined(_GLIBCXX_USE_CXX11_ABI) // See PR #5439.
# if defined(__NVCOMPILER)
// // Assume that NVHPC is in the 1xxx ABI family.
// // THIS ASSUMPTION IS NOT FUTURE PROOF but apparently the best we can do.
// // Please let us know if there is a way to validate the assumption here.
# elif !defined(__GXX_ABI_VERSION)
# error \
"Unknown platform or compiler (_GLIBCXX_USE_CXX11_ABI): PLEASE REVISE THIS CODE."
# endif
# if defined(__GXX_ABI_VERSION) && __GXX_ABI_VERSION < 1002 || __GXX_ABI_VERSION >= 2000
# error "Unknown platform or compiler (__GXX_ABI_VERSION): PLEASE REVISE THIS CODE."
# endif
# define PYBIND11_BUILD_ABI \
"_libstdcpp_gxx_abi_1xxx_use_cxx11_abi_" PYBIND11_PLATFORM_ABI_ID_TOSTRING( \
_GLIBCXX_USE_CXX11_ABI)
# else
# error "Unknown platform or compiler: PLEASE REVISE THIS CODE."
# endif
#endif

#ifndef PYBIND11_INTERNALS_KIND
# define PYBIND11_INTERNALS_KIND ""
// On MSVC, debug and release builds are not ABI-compatible!
#if defined(_MSC_VER) && defined(_DEBUG)
# define PYBIND11_BUILD_TYPE "_debug"
#else
# define PYBIND11_BUILD_TYPE ""
#endif

#define PYBIND11_PLATFORM_ABI_ID \
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \
PYBIND11_BUILD_TYPE
PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI PYBIND11_BUILD_TYPE
4 changes: 2 additions & 2 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,11 @@ struct type_info {

#define PYBIND11_INTERNALS_ID \
"__pybind11_internals_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \
PYBIND11_PLATFORM_ABI_ID "__"
PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE PYBIND11_PLATFORM_ABI_ID "__"

#define PYBIND11_MODULE_LOCAL_ID \
"__pybind11_module_local_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \
PYBIND11_PLATFORM_ABI_ID "__"
PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE PYBIND11_PLATFORM_ABI_ID "__"

/// Each module locally stores a pointer to the `internals` data. The data
/// itself is shared among modules with the same `PYBIND11_INTERNALS_ID`.
Expand Down
Loading