Expression refactor - #58
Merged
Merged
Conversation
Build if_, switch_ and while_ by chaining one piece at a time:
if_(c1).then_(a).else_if_(c2).then_(b).else_(d)
switch_(v).case_(10).then_(a).default_(d)
while_(c).do_(body)
Each construct alternates between a pending builder -- which has no
operator(), is not Deferred, and is [[nodiscard]] -- and a complete
expression. The two-argument entry points, the variadic switch_, append()
and the free case_()/default_() factories are replaced by the chain;
else_if is renamed else_if_ for a uniform trailing underscore.
The switch no longer stores its default at case index 0. It lives in its
own member behind a no_default tag, mirroring no_else, so a case may be
added to a switch that already has a default: it is checked after the
cases already present and before the default, preserving what append()
did. A switch without default_ is now a usable expression yielding a
std::optional, like an if_ without else_.
Also fixes three pre-existing defects in result-type deduction, which
used the raw operator() return type instead of what evaluate() yields:
- a branch or case body returning a deferred expression produced
variant<expression_<...>, T> instead of T;
- lvalue constant branches deduced T const& bound to a temporary,
which segfaulted under AddressSanitizer;
- lvalue variable branches failed to compile at all.
detail::evaluated_result_t is the single spelling of "what evaluating
this yields"; the deducers now use it. An all-void switch is no longer
reported as potentially throwing, and while_builder's forwarding
constructor no longer hijacks its own copy constructor.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CftmcRq9o1fztk1puisTNN
Owner
Author
std::optional's converting constructor has no noexcept specification in the standard; libstdc++ adds one and libc++ does not. Evaluating a non-finalized conditional goes through both the no-match path (optional from nullopt_t, always noexcept) and the matched-branch path (optional from int), so equating noexcept(ex()) with no_match_is_nothrow_v alone held only on libstdc++ and MSVC, and failed the macOS build. Assert against both halves, matching the caveat already documented for the same reason in test/unit/conditional.cpp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CftmcRq9o1fztk1puisTNN
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 371e87712b
ℹ️ 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".
conditional_builder::then_'s const& overload materializes Condition(m_condition) -- a copy -- before calling append_conditional, but its specification only tested construction from Condition&&, a move. A condition whose copy throws and whose move does not was reported as noexcept, so a throwing copy would terminate instead of propagating. Include construction from Condition const& in the specification. This is the only overload that copies a member into an argument rather than passing it through; the rest already spell their const& types, and all fourteen builder overloads were re-measured against a throwing-copy, nothrow-move type. Reported by chatgpt-codex-connector on #58. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CftmcRq9o1fztk1puisTNN
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
1 participant
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.
No description provided.