Repository navigation
Add C++26 saturation arithmetic polyfills (#39) - #255
thirtytwobits wants to merge 4 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a530f449e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses the Codex review of #255. The shared integer traits in cetl/pf20/utility.hpp now ignore cv-qualifiers, so explicit template arguments such as saturating_add<const bool> or cmp_less<const char, char> are rejected exactly like their unqualified forms, while const int is accepted on every implementation path. GCC's __int128 is now supported in the strict -std=c++NN modes CETLVaSt builds with, where std::is_integral, std::is_signed, std::make_unsigned and std::numeric_limits do not describe it: the traits carry their own signedness, unsigned counterpart and two's-complement limits for the 128-bit types, and cetl/pf26/numeric.hpp uses those traits throughout instead of the standard ones. The forwarders used when the standard library only has the P0543R3 names get trailing return types that depend on the standard call, so they participate in overload resolution exactly when the standard function would. Tests: cv-qualified explicit template arguments; 128-bit constant expressions for the comparison functions, in_range and all five saturation functions including the portable implementations; a compile-failure test for const bool. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
MAMONTBOT
left a comment
There was a problem hiding this comment.
Reviewed the changes; no blocking issues found.
MAMONTBOT
left a comment
There was a problem hiding this comment.
Reviewed the changes; no blocking issues found.
libcyphal and nunavut both need saturating integer operations, and C++26 standardized them (P0543R3, renamed by P4052R0 to saturating_add, saturating_sub, saturating_mul, saturating_div and saturating_cast). CETL now provides those five functions for C++14 and newer, exact to the standard: one operand type that is never promoted, bool and the character types excluded, a y != 0 precondition on division, and min / -1 saturating to max. The compiler's overflow builtins are used where present and a portable implementation that never forms an overflowing expression otherwise; both are constant expressions in C++14 and both are tested. cetl/pf26/cetlpf.hpp selects the implementation by the library feature-test macro rather than __cplusplus, because current compilers still report an interim value for -std=c++26 and because the functions were renamed late: 202603L uses the standard names directly, 202311L forwards to the earlier std::add_sat family under the final names, and anything else uses cetl::pf26. saturating_cast needs sign-correct comparisons, so the C++20 integer comparison functions (cmp_equal, cmp_less, ..., in_range) are added as cetl::pf20 polyfills, and nunavut's "saturated intN" fields need a clamp to an arbitrary range, so std::clamp is added as a cetl::pf17 polyfill. Both are wired into their cetlpf.hpp headers. Tests: exhaustive 8-bit and sampled 16-bit operand pairs, random 32/64-bit operands against a 128-bit oracle, exhaustive saturating_cast from the 8- and 16-bit types into every fixed-width type, an integer-comparison sweep against a sign-aware oracle, debug-assert death tests, compile-failure tests for bool and mixed operand types, and a differential libFuzzer target. A new cpp-26 preset is generated for every toolchain except GCC 7 and added to the amd64 CI matrix so the standard-library paths are exercised. Example 13 and CETL_CPP_STANDARD_23/26 document the addition. Refs #39; the cetl::saturating<T> wrapper type follows in a separate PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Addresses the Codex review of #255. The shared integer traits in cetl/pf20/utility.hpp now ignore cv-qualifiers, so explicit template arguments such as saturating_add<const bool> or cmp_less<const char, char> are rejected exactly like their unqualified forms, while const int is accepted on every implementation path. GCC's __int128 is now supported in the strict -std=c++NN modes CETLVaSt builds with, where std::is_integral, std::is_signed, std::make_unsigned and std::numeric_limits do not describe it: the traits carry their own signedness, unsigned counterpart and two's-complement limits for the 128-bit types, and cetl/pf26/numeric.hpp uses those traits throughout instead of the standard ones. The forwarders used when the standard library only has the P0543R3 names get trailing return types that depend on the standard call, so they participate in overload resolution exactly when the standard function would. Tests: cv-qualified explicit template arguments; 128-bit constant expressions for the comparison functions, in_range and all five saturation functions including the portable implementations; a compile-failure test for const bool. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
e433f70 to
4dc1c3f
Compare
The release tooling's required_jobs() must list every CETLVaSt matrix entry, and its contract test fails when the workflow matrix and the list disagree. The cpp-26 standard added to the amd64 matrix was missing from the list, which failed release-tests and therefore release-ready on #255. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Addresses the Codex review of #255. The shared integer traits in cetl/pf20/utility.hpp now ignore cv-qualifiers, so explicit template arguments such as saturating_add<const bool> or cmp_less<const char, char> are rejected exactly like their unqualified forms, while const int is accepted on every implementation path. GCC's __int128 is now supported in the strict -std=c++NN modes CETLVaSt builds with, where std::is_integral, std::is_signed, std::make_unsigned and std::numeric_limits do not describe it: the traits carry their own signedness, unsigned counterpart and two's-complement limits for the 128-bit types, and cetl/pf26/numeric.hpp uses those traits throughout instead of the standard ones. The forwarders used when the standard library only has the P0543R3 names get trailing return types that depend on the standard call, so they participate in overload resolution exactly when the standard function would. Tests: cv-qualified explicit template arguments; 128-bit constant expressions for the comparison functions, in_range and all five saturation functions including the portable implementations; a compile-failure test for const bool. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The release tooling's required_jobs() must list every CETLVaSt matrix entry, and its contract test fails when the workflow matrix and the list disagree. The cpp-26 standard added to the amd64 matrix was missing from the list, which failed release-tests and therefore release-ready on #255. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The release tooling's required_jobs() must list every CETLVaSt matrix entry, and its contract test fails when the workflow matrix and the list disagree. The cpp-26 standard added to the amd64 matrix was missing from the list, which failed release-tests and therefore release-ready on #255. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Summary
First of the two PRs agreed in #39 (proposal, decisions). It adds the C++26 saturation arithmetic as a true polyfill back to C++14, plus the two smaller standard facilities the consumers need around it. The
cetl::saturating<T>wrapper type follows in a separate PR.cetl/pf26/numeric.hpp:cetl::pf26::saturating_add,saturating_sub,saturating_mul,saturating_div,saturating_cast, exact to[numeric.sat]under the final P4052R0 names: one operand type that is never promoted,booland the character types excluded,y != 0as a precondition on division (CETL_DEBUG_ASSERT,CDE_numeric_001), andmin / -1saturating tomax.__builtin_*_overflowis used where the compiler has it (GCC 5+, Clang); otherwise a portable implementation that never forms an overflowing expression. Both are constant expressions in C++14 and both are compiled and tested on every toolchain.cetl/pf26/cetlpf.hpp: selects the implementation by__cpp_lib_saturation_arithmetic, not__cplusplus(every current compiler still reports202400Lfor-std=c++26):>= 202603Lusesstd::saturating_*directly;>= 202311L(libstdc++ 14/15, libc++ 18–22) forwards tostd::add_sat/std::saturate_castunder the final names; otherwisecetl::pf26.cetl/pf20/utility.hpp:cmp_equal,cmp_not_equal,cmp_less,cmp_greater,cmp_less_equal,cmp_greater_equal,in_range(C++20 polyfills;saturating_castneeds sign-correct comparisons). Wired intocetl/pf20/cetlpf.hpp.saturating_add<const bool>is rejected likebool) and support GCC's__int128in the strict-std=c++NNmodes where the standard traits andstd::numeric_limitsdo not describe it.cetl/pf17/algorithm.hpp:clamp(C++17 polyfill; nunavut'ssaturated intNfields clamp to an arbitrary range). Wired intocetl/pf17/cetlpf.hpp.cetl/cetl.hpp:CETL_CPP_STANDARD_23/CETL_CPP_STANDARD_26and the example 13 page.cpp-26presets generated with tcpm for every toolchain except GCC 7,cpp-26added to the amd64 CI matrix (in ts26.4.5 both g++ 15 and clang 22 take the202311Lbranch, so CI exercises the adapted-std path; the polyfill path is covered by C++14/17/20/23).Tests
test_pf26_numeric.cpp: constant-expression cases; every operand pair forint8_t/uint8_t; every first operand ofint16_t/uint16_tagainst edge and sampled second operands; edge pairs and 100k random pairs for the 32-bit types against along longoracle and for the 64-bit types against a 128-bit oracle (where available);saturating_castfrom every value of the 8- and 16-bit types into every fixed-width type; 64-bit edge tables; the portable implementations checked alongside the builtin ones; thecetlpf.hppselection; a death test for division by zero.test_pf20_utility.cpp: the classic mixed-sign cases as constant expressions and a sweep over every pair of fixed-width types against a sign-aware oracle.test_pf17_algorithm.cpp: value, reference-identity, comparator and floating-point cases, and a death test for inverted bounds (CDE_algorithm_001).booloperands and mixed operand types forsaturating_add,boolforcmp_less.fuzz_saturation_arithmetic.cpp: differential libFuzzer target against a 128-bit oracle for all five functions and every fixed-width type, with a seed corpus; the fuzz CMake now loops over its targets.example_13_saturating_arithmetic.cpp: counter saturation, uptime narrowing, explicit operand type,clampandin_range.Validation
cetl::pf26polyfill (builtins and portable)run_unittests+run_examplespass; compile-failure tests 9/9std::saturating_*(202603L)std::add_satfamily adapted (202311L)generate_CETL_docspf26/cetlpf.hppsnippets renderNot covered locally: Apple clang in C++26 mode (CMake 4.1 has no
CXX26flags for AppleClang); the pre-rename libc++ branch was only exercised with a standalone build during the design phase.Refs #39.
🤖 Generated with Claude Code