Repository navigation
Add C++23 expected polyfill (#13) - #256
thirtytwobits wants to merge 7 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: ecdeaf0299
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d163fb2d0e
ℹ️ 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".
|
@codex review |
ab57319 to
041b7f8
Compare
|
Codex Review: Didn't find any major issues. Nice work! 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.
041b7f8 to
5c10022
Compare
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>
libcyphal wants cetl::expected (its types.hpp aliases Expected<S, F> to cetl::variant<S, F> as a stand-in) and embedded code built without exceptions needs a way to return a value or the reason there is none. C++23 standardized that as std::expected; CETL now provides it for C++14 and newer as cetl::pf23::expected, unexpected, unexpect_t/unexpect and bad_expected_access, including expected<void, E> and the monadic operations (and_then, or_else, transform, transform_error). The polyfill follows the current standard, including the defect reports that libraries apply in C++23 mode: LWG 3836 (expected<bool, E> from another expected), LWG 4222 (the value constructor excludes unexpect_t), P3379R0 (the equality operators are constrained rather than mandated), and remove_cv_t<T> as the default template argument of the value constructor and value_or. Assignments that switch between the value and the error give the strong exception guarantee the standard specifies. Copy/move construction and destruction are trivial when T and E are; assignment is too, which the standard leaves unspecified. Without exceptions value() calls std::terminate, as cetl::pf17::optional does. Unchecked access (operator*, operator->, error()) has debug assertions. Known differences, documented on the class: in C++14 the in-place constructors take cetl::pf17::in_place_t (std::in_place_t from C++17 on, which is what cetl::in_place is there), members that need placement new are not constexpr, and transform() needs a movable result because C++14 has no guaranteed copy elision; cv-qualified void is not supported as the value type. cetl/pf23/cetlpf.hpp selects std::expected when __cpp_lib_expected >= 202211L (the monadic operations) and the polyfill otherwise, and cetl/pf26/cetlpf.hpp now includes it so the polyfill headers chain. Tests are typed over the implementation: where the standard library has std::expected every behavioural test also runs against it, which checks the expectations as well as the polyfill. libstdc++ 13 predates LWG 3836, so that one oracle check is skipped where the defect is detected. Coverage of expected.hpp is 99% of lines and 79% of branches (GCC 13, C++14). There are compile-failure tests for the mandates, death tests for the debug assertions and for value() without exceptions, and example 14 for the documentation. Stacked on the issue #39 branch, which adds the C++23/26 presets that exercise the std::expected path in CI. Closes #13. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assignment, emplace(), swap() and the constructors that convert from another expected change which member of the storage union is active. C++20 allows that in a constant expression (std::construct_at and constexpr destructors), and std::expected is constexpr throughout, so the polyfill now marks those operations constexpr when the language and library support it (__cpp_constexpr_dynamic_alloc and __cpp_lib_constexpr_dynamic_alloc). Before C++20 they cannot be constant expressions; the class documentation now says "before C++20" rather than "in C++14". A typed test runs every such operation in a constant expression, for a trivial and a non-trivial literal type, so in C++23 builds it is also checked against std::expected. A compile-failure test pins that value() && still requires a copy constructible error type: [expected.object.obs] mandates it for the rvalue overloads too, and libc++ 23 and libstdc++ 15 reject a move-only error type the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The standard returns the comparison itself from the equality operators of expected and unexpected, and the argument itself from error_or, so the conversion applied is the implicit one that the constraints check. The polyfill used static_cast instead, which can pick a different, explicit conversion: a comparison result with a better-matching explicit operator bool gave the opposite answer (or did not compile if that operator was deleted), and error_or could select an explicit constructor of E over the implicit conversion from G. The comparisons now return the comparison result, as LWG 4366 (C++26) specifies for expected and as C++23 already specifies for unexpected, and error_or returns its forwarded argument. libc++ 23 behaves the same way; libstdc++ 13 and 15 still cast in the expected comparisons, so that oracle check is skipped where the cast is detected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
[expected.un.swap] mandates is_swappable_v<E> for the member swap and constrains the friend swap on it. libc++ follows that wording; libstdc++ constrains the member swap as well, so code that only asks whether x.swap(y) is callable gets different answers. The polyfill keeps the standard's wording. A typed test now checks, against libc++ and libstdc++ as well as the polyfill, the parts they agree on: the friend swap is constrained, so the swappable traits follow E (including nothrow), and swapping works for swappable error types. A compile-failure test checks that calling the member swap with a non-swappable error does not compile, which holds under either reading. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5c10022 to
793f7b3
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>
Summary
Adds
cetl::pf23::expected<T, E>(includingexpected<void, E>),unexpected<E>,unexpect_t/unexpectandbad_expected_access<E>for C++14 and newer, pluscetl/pf23/cetlpf.hpp. Closes #13.expected<bool, E>from another expected), LWG 4222 (value ctor excludesunexpect_t), P3379R0 (equality operators are constrained),remove_cv_t<T>default template args.and_then,or_else,transform,transform_errorin all four value categories, using a detail-only C++17INVOKE(callables, member function/data pointers,reference_wrapper). In C++17+transformconstructs its result in place (guaranteed elision).swapthat switch between value and error (reinit-expected), exactly as specified; assignment is deleted unless T or E is nothrow-move-constructible.value()on an error callsstd::terminate(aspf17::optionaldoes). Unchecked access (*,->,error()) hasCETL_DEBUG_ASSERTs (CDE_expected_001–015).cetl/pf23/cetlpf.hppusesstd::expectedwhen__cpp_lib_expected >= 202211L(has the monadic ops), else the polyfill.cetl/pf26/cetlpf.hppnow includes it so the polyfill headers chain.Documented differences (C++14 language limits / scope): in C++14 the in-place ctors take
cetl::pf17::in_place_t(from C++17 onstd::in_place_t, which is whatcetl::in_placeis there); members needing placement new aren'tconstexprin C++14;transformneeds a movable result in C++14; cv-qualifiedvoidvalue type is not supported (plainvoidis).Tests
test_pf23_expected.cpp(type properties, construction, assignment, emplace, swap, exception safety) andtest_pf23_expected_operations.cpp(observers,bad_expected_access, monadic ops, comparison,unexpected,cetlpf.hpp) are typed over the implementation: where the library hasstd::expected, every behavioural test also runs against it, so the expectations are checked against a real standard library too. libstdc++ 13 predates LWG 3836; that one oracle check is skipped only when the defect is detected at runtime.test_pf23_expected_asserts.cpp: death tests for the debug assertions and forvalue()with-fno-exceptions.and_thenreturning a non-expected,transformreturning a reference,unexpected<int>as value type.example_14_expected.cpp) for the docs.Validation
run_unittests+run_examples, GCC 13 C++14 Debug): all 71 executables pass. pf26 suites pass under g++-15 C++26 with the new include chain.expected.hpp(GCC 13, C++14): 99% lines, 79% branches.bad_expected_access<void>(same as the existing one intype_traits_ext.hpp;WARN_AS_ERROR=NO).main, CETLVaSt does not trigger on it automatically; it was dispatched manually on the branch and every job passed, includinggcc7-nativeC++14 (with clang-format 19), thegcc-native-32builds, C++26 and docs: run 39805479435.🤖 Generated with Claude Code