Update C++ Standards to allow C++23#609
Open
ChrisJChang wants to merge 5 commits into
Open
Conversation
Build system:
- Rewrite the root and gum/ CMake standard-detection ladders to try
C++23 -> C++20 -> C++17, and fail hard if even C++17 is unavailable
(previously fell back through C++14/C++11/C++0x).
- Extend check_root_std_flag()'s recognised standard list so it can
still detect a ROOT installation built with C++20/C++23 (or the
pre-standardisation 2a/2b flag spellings) and downgrade GAMBIT to
match; a ROOT built with pre-C++17 now triggers a clear error.
- Generalize the gm2calc/FastJet backend "downgrade to C++14" regexes,
which previously only matched the literal "-std=c++17" substring and
would silently stop working (compiling those backends with whatever
newer standard GAMBIT picked) once GAMBIT's own default moved past
C++17.
- Remove the now-vacuous "c++14" tag from check_ditch_status, since
GAMBIT always compiles with at least C++17.
- Bump the minimum required Eigen3 version to 3.3.7, since older Eigen
releases use STL idioms (std::unary_function/binary_function,
std::result_of, etc.) that are deprecated or removed in C++17/20/23.
Source fixes:
- Replace `throw()` dynamic exception specifications with `noexcept`
in Utils/exceptions.{hpp,cpp} and ColliderBit's Py8Collider.hpp;
dynamic exception specs were removed in C++20 and would otherwise be
a hard compile error.
- Patch the vendored yaml-cpp 0.6.2's node_iterator_base to stop
deriving from std::iterator<...>, which was removed in C++20, and
provide the iterator typedefs directly instead.
Verified by configuring and building under GCC 13 (which now defaults
to -std=c++23): CMake configure completes end to end, and both the
patched yaml-cpp library and several Utils translation units compile
successfully.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EDpAzhpyYk2biu7ttb9EuQ
The previous commit's comments and error messages called out that C++11/C++14 are "no longer supported", but most users and developers never knew or cared that GAMBIT once supported them. Reword the standard-detection comments and FATAL_ERROR messages to just state the current C++17+ requirement, without the historical aside. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EDpAzhpyYk2biu7ttb9EuQ
ra_copy_hdf5::run is instantiated once per printable type, including std::string, and filled its output buffer with std::vector<U> output(size, 0). For U = std::string, the literal 0 is a null pointer constant, and libstdc++ added a deleted basic_string(nullptr_t) overload in C++23 (P2166) specifically to catch this pattern, turning what used to be a silent (and already dubious) null-pointer construction into a hard compile error under -std=c++23. Use std::vector<U> output(size) instead, which value-initializes each element (0 for arithmetic types, "" for std::string) without relying on any implicit conversion from 0, and behaves identically to the old code for every type this function is actually instantiated with. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EDpAzhpyYk2biu7ttb9EuQ
Farray's "copy constructor" took a non-const lvalue reference
(Farray<T,lims...>&) instead of a const reference. Through C++20, the
two-stage implicit-move rule for "return localVar;" first tries the
operand as an rvalue (fails here, since nothing can bind an rvalue to
a non-const lvalue ref) and then retries the same overload resolution
treating it as an lvalue, which succeeds by calling this constructor.
C++23's simpler implicit-move rules (P2266) fold this into a single
overload resolution pass instead of a separate lvalue retry, so a
constructor that can only bind non-const lvalues is no longer found
when returning a named local Farray by value, e.g.:
Backends/src/frontends/Prospino_2_1.cpp:279:
error: cannot bind non-const lvalue reference of type
'Gambit::Farray<double, 0, 99>&' to an rvalue of type
'Gambit::Farray<double, 0, 99>'
Since Farray is used throughout the backend frontends specifically for
returning Fortran-array-compatible values by value, this affects every
translation unit that returns an Farray by name, not just Prospino.
Fix: give Farray a proper const-reference copy constructor. This adds
no new members and doesn't change the class's memory layout (which
must exactly match Fortran array layout), just the constructor
signature, so it's a pure C++-semantics fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EDpAzhpyYk2biu7ttb9EuQ
ClosestPair2D declares "class Point;" as a forward declaration and
holds a std::vector<Point> _points member, with Point's full
definition only appearing later in the file, out-of-class. Its two
constructors were defined inline, in the class body. Per the
complete-class-context rule, inline member function bodies are
semantically analyzed immediately after the class's own closing brace
-- before Point's separate out-of-class definition -- so the implicit
default-construction of the incomplete-at-that-point _points member
(as part of the constructors' implicit member-initialization) is only
valid because GCC is lenient about exactly when it enforces this.
Clang enforces it strictly, and only starting with -std=c++20/c++23
(building with clang++ 18 against libstdc++ 13/14):
fjcore.cc:729:3: note: in defaulted default constructor for
'std::vector<gambit::fjcore::ClosestPair2D::Point>' first
required here
.../bits/stl_vector.h:370: error: arithmetic on a pointer to an
incomplete type 'gambit::fjcore::ClosestPair2D::Point'
Fix: declare the two constructors in the class body without inline
bodies, and define them out-of-line after Point's definition, exactly
matching the pattern this same file already uses for _ID() and
size(). Verified clean with clang++ and g++ under -std=c++17/20/23.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EDpAzhpyYk2biu7ttb9EuQ
Member
|
The fjcore problem seems to be fixed in newer versions of fastjet, see version comment for fastjet 3.4.2 here: Maybe we should take the hit and update our fastjet/fjcore versions as well in this PR? (I am being very demanding now :P ) Same might be true for the yaml issue. Did we not discuss/ actually update this relatively recently, or have I just been dreaming? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This raises the minimum C++ standard to C++17, and adds some bug fixes to allow gambit to build with C++20 and C++23.
It also raises the minimum Eigen version from v3.1.0 to v3.3.7.
This still allows backends to use lower standards when they do not support higher ones, e.g. AltarBBN.
I have tested with gcc=14, and clang 18.