Require C++17 and fix the CI standard matrix (after #112) - #126
Merged
Conversation
C++17 added std::size and std::data. Arguments whose types carry std template arguments (std::vector, mcdata<std::vector<T>>, ...) pull namespace std in via ADL, making unqualified size()/data() calls that previously resolved to alps::size/alps::data ambiguous. Qualify the call sites in alea/mcanalyze.hpp, alea/mcdata.hpp, numeric/vector_valarray_conversion.hpp, and test/alea/mcanalyze.C. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
non_pod's mixed-type friends delegated T==non_pod to non_pod==T, which under C++20 operator rewriting resolves back to the same friend as a reversed candidate: infinite recursion (a hang at -O3, stack overflow at -O0). test_deque/test_vector timed out at 600 s in the first real C++23 CI run. Compare data_ directly instead. Verified with g++ 16 -std=c++23 -O3: both tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI has been sweeping C++11-23 by passing -std=c++XX via CMAKE_CXX_FLAGS, but the unconditional set(CMAKE_CXX_STANDARD 14) appended its own -std flag afterwards, so every job silently built C++14. CONTRIBUTING.md has claimed C++17 all along; this makes the build match the docs. - CMakeLists.txt now defaults CMAKE_CXX_STANDARD to 17 and honors -DCMAKE_CXX_STANDARD=20/23; CI passes the standard that way instead of through CMAKE_CXX_FLAGS. The sweep tests 20/23 on top of the 17 baseline (the 11/14 entries are gone with the floor raise). - Remove ' throw (std::runtime_error)' exception specifications in src/ietl/krylov_wrapper.h (ill-formed since C++17) and the dead, never-included src/boost/function_objects.hpp (std::binary_function was removed in C++17). - Drop GCC 10 / Clang 13 from the matrix; GCC 11 / Clang 14 (Ubuntu 22.04 / RHEL 9 defaults) are the new tested floor. - Delete requirements.txt: unreferenced, and its numpy<2.1 ceiling contradicted pyproject.toml and CI reality. Verified on the skilledwolf/ALPS fork: full builds green at C++17/20/23 with GCC 11-16 and Clang 14-22. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
skilledwolf
marked this pull request as ready for review
August 18, 2026 21:37
Contributor
|
I know I'm late to the party – but is there a good reason to disallow C++-14, i.e. any feature that requires C++-14? If not, can we allow it again and lower the floor back down to C++-14? |
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
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.
Summary
Raise the minimum C++ standard from C++14 to C++17 and ensure CI builds with the standard selected by each matrix job.
Previously, CI passed
-std=c++XXthroughCMAKE_CXX_FLAGS, but the project’s unconditionalCMAKE_CXX_STANDARD=14took precedence. As a result, the C++17/20/23 jobs all built as C++14.Changes
Verification
Verified C++17, C++20, and C++23 builds across GCC 11–16 and Clang 14–22 on my fork.