Fix STLAllocator compatibility with GCC 15 and modern C++ standards#568
Fix STLAllocator compatibility with GCC 15 and modern C++ standards#568wentasah wants to merge 1 commit intoOGRECave:v2-3from
Conversation
Updated STLAllocator constructors to be compatible with modern C++
standard library containers:
1. Made constructors constexpr: Added constexpr keyword to all
constructors to make them compatible with modern C++ standard library
containers that require constexpr allocators.
2. Removed explicit from default constructor: The default constructor
was marked as explicit, which prevented it from being used in
implicit initialization contexts (like {}-initialization). Standard
library containers expect allocators to have non-explicit default
constructors.
3. Conditionally made destructor constexpr: Added a preprocessor check
to only make the destructor constexpr in C++20 and later, since
constexpr destructors are not available in earlier C++ standards.
These changes fix compilation errors with GCC 15 where the hashtable
implementation tries to use {} initialization on the allocator member,
which requires a non-explicit constexpr default constructor.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
What's the take from Gazebo team? They don't like changes that break the ABI and this almost certainly an ABI-breaking change (I didn't run the abi checker to confirm). There was a similar discussion with #541 where Apple broke support. Fortunately we avoided it entirely because the affected component wasn't actually in use. Given that at this point the v2-3 branch is basically "whatever gazebo is doing", it may be solvable by simply doing |
|
@scpeters Can you answer the above? We're using this patch in nix-ros-overlay and it works fine, but ABI compatibility is not that important in Nix ecosystem as elsewhere, and I have no clue how and where is this used. |
I will discuss with the gazebo team |
Thanks for checking. Given our model of loading plugins for gz-rendering, we can probably deal with an ABI breakage without too much problem for the end users that load the gz-rendering-ogre2 plugin directly from our binaries. Said that, I known that we declared the Semantic Versioning starting with 3.x but might be a good practice try to keep it stable if we can. For this case, we can potentially add a compiler flag (default to OFF) that brings the patches in this PR (something like -DUSE_MODERN_CXX) so the people that needs the patch enable it in their building metadata while the software maintains the ABI stability for all the 2.3 potential consumers out there. |
Changes: * added `fix_stlallocator_gcc15.patch` patch (from OGRECave/ogre-next#568) that fixes compilation with gcc 15 * changed `OGRE_SET_DISABLE_JSON` to `0` in `BUILD.bazel` to enable features that require json, e.g. `VctVoxelizer`. The `ogre-main` target links against `rapidjson` already so this change just flips the switch. * added `load(..)` statements for `cc_library` and `cc_binary` so it's compatible with bazel version 9.x Signed-off-by: Ian Chen <iche@intrinsic.ai>
I'm fine with adding |
We have been using it for the last decade in Gazebo and it we did not have any serious problem that it was not detected. So it is old but still works I would say. Looking now more into the changes i think that they affect to the source semantics but not to the binary layer generated to my eyes. |
4 participants
Updated STLAllocator constructors to be compatible with modern C++ standard library containers:
Made constructors constexpr: Added constexpr keyword to all constructors to make them compatible with modern C++ standard library containers that require constexpr allocators.
Removed explicit from default constructor: The default constructor was marked as explicit, which prevented it from being used in implicit initialization contexts (like {}-initialization). Standard library containers expect allocators to have non-explicit default constructors.
Conditionally made destructor constexpr: Added a preprocessor check to only make the destructor constexpr in C++20 and later, since constexpr destructors are not available in earlier C++ standards.
These changes fix compilation errors with GCC 15 where the hashtable implementation tries to use {} initialization on the allocator member, which requires a non-explicit constexpr default constructor.
Note: this fixes the same problem as #564, but was entirely AI generated. I'm not sure whether you have some AI policy so feel free to disregard it if it violates your policy.