Skip to content

Commit

Permalink
PYBIND11_PLATFORM_ABI_ID Modernization Continued (platforms other t…
Browse files Browse the repository at this point in the history
…han MSVC) (#5439)

* THIS IS JUST A START: First attempt to combine information from PR #4953 and PR #5437

* Include GXX_ABI and USE_CXX in the identifier

Further constrain to GXX_ABI 1002 or greater and less than 2000,
hopefully future proof by summarizing that as `1` along with CXX11 on or
off.

* style: pre-commit fixes

* Use `gxx_abi_1xxx` and simplify the Clang string

After discussions with Ralf Grosse-Kunstleve we think these would make
good identifiers that are concise and clear.

* Error if `_GLIBCXX_USE_CXX11_ABI` is not defined

Within the `__GXX_ABI_VERSION` block this should always be defined,
guard against unexpected defines and make the error obvious.

* Change `usecxx11` to `use_cxx11_abi` for correspondence with `_GLIBCXX_USE_CXX11_ABI` (similarly to `gxx_abi` for `__GXX_ABI_VERSION`).

* `PYBIND11_COMPILER_TYPE` overhaul, mainly: replace `_icc`, `_clang`, `_gcc` with `_system`

* Add NVHPC (__PGI) to the list of compilers compatible with system compiler.

See comment by @robertmaynard: #5439 (comment)

* Fix oversight: remove __NVCOMPILER elif branch in PYBIND11_BUILD_ABI block.

Also add comment pointing to this PR (#5439).

* Revert "Fix oversight: remove __NVCOMPILER elif branch in PYBIND11_BUILD_ABI block."

This reverts commit d412303.

* Revert "Add NVHPC (__PGI) to the list of compilers compatible with system compiler."

This reverts commit 9fc9515.

* Define NVHPC PYBIND11_BUILD_ABI using __GNUC__, __GNUC_MINOR__, _GLIBCXX_USE_CXX11_ABI

* Use _GLIBCXX_USE_CXX11_ABI to detect libstdc++, then assume that NVHPC is always in the 1xxx ABI family.

* Enhance NVHPC comment and limited future proofing.

* The `PYBIND11_STDLIB` is obsolete but kept around to maintain backward compatibility.

* Move `PYBIND11_BUILD_TYPE` down in the file, so that the order of macro definitions is the same as in the list defining `PYBIND11_PLATFORM_ABI_ID`

* Introduce `PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE`:

This makes it possible to achieve these two goals:

* Avoid the leading underscore in `PYBIND11_PLATFORM_ABI_ID` (see #5439 (comment))

* Maintain backward compatibility for use cases as reported under #5439 (comment)

`PYBIND11_INTERNALS_KIND` is removed in this commit to ensure that `PYBIND11_COMPILER_TYPE` is the first element of the `PYBIND11_PLATFORM_ABI_ID`, so that `PYBIND11_COMPILER_TYPE_LEADING_UNDERSCORE` can meaningfully be used as a prefix for `PYBIND11_PLATFORM_ABI_ID` in pybind11/detail/internals.h.

* Apply suggestion by @isuruf, with revised comments (code is as suggested).

* Make determination of `PYBIND11_COMPILER_TYPE` `"macos"` or `"glibc"` more general.

The main motivation is to resolve these "Manylinux on 🐍 3.13t • GIL" and "Pyodide wheel" failures:

```
/__w/pybind11/pybind11/include/pybind11/conduit/pybind11_platform_abi_id.h:35:10: error: #error "Unknown PYBIND11_COMPILER_TYPE: PLEASE REVISE THIS CODE."
   35 | #        error "Unknown PYBIND11_COMPILER_TYPE: PLEASE REVISE THIS CODE."
      |          ^~~~~
```

(All other CI jobs succeeded.)

Further thought:

Essentially, under Linux and macOS the `PYBIND11_COMPILER_TYPE` is only for informational purposes.
ABI compatibility is determined by the libstdc++ or libc++ ABI version.

* Add `PYBIND11_COMPILER_TYPE` `emscripten`

* Add `PYBIND11_COMPILER_TYPE` `graalvm`

* Revert "Add `PYBIND11_COMPILER_TYPE` `graalvm`"

This reverts commit 75da5fb.

* Revert "Add `PYBIND11_COMPILER_TYPE` `emscripten`"

This reverts commit e34dc8b.

* Revert "Make determination of `PYBIND11_COMPILER_TYPE` `"macos"` or `"glibc"` more general."

This reverts commit 41daaa4.

* Revert "Apply suggestion by @isuruf, with revised comments (code is as suggested)."

This reverts commit ca9e699.

* Remove `defined(__INTEL_COMPILER)` as suggested by @hpkfft under #5439 (comment)

---------

Co-authored-by: Marcus D. Hanwell <marcus@cryos.net>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 20, 2024
1 parent 741d86f commit 3e41948
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 43 deletions.
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(__clang__) || defined(__GNUC__)
# define PYBIND11_COMPILER_TYPE "system" // Assumed compatible with system compiler.
# 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

0 comments on commit 3e41948

Please sign in to comment.