Skip to content

If condition - #46

Merged
ipapadop merged 15 commits into
mainfrom
if-condition
May 25, 2026
Merged

If condition#46
ipapadop merged 15 commits into
mainfrom
if-condition

Conversation

@ipapadop

@ipapadop ipapadop commented May 5, 2026

Copy link
Copy Markdown
Owner

No description provided.

@ipapadop ipapadop self-assigned this May 5, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the if_then deferred expression and a homogenized_type trait to handle mixed return types using std::variant. It also refactors several expressions to use named members and adds Deferred concept constraints. Feedback highlights regressions in visit methods that break recursive traversal, suggests adding non-const operator() overloads for stateful callables, and recommends using std::decay_t for result types. Additionally, the reviewer suggests adopting the new homogenized_type_t trait in switch_expression for better type safety.

Comment thread include/deferred/conditional.hpp Outdated
Comment thread include/deferred/conditional.hpp Outdated
Comment thread include/deferred/conditional.hpp Outdated
constexpr explicit default_expression(T&& t) : m_expression(std::forward<T>(t))
{ }

[[nodiscard]] constexpr decltype(auto) operator()() const

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The default_expression class is missing a non-const operator(). This prevents the use of mutable lambdas or other stateful callables as the default case in a switch_ expression.

  [[nodiscard]] constexpr decltype(auto) operator()()
  {
    return evaluate(m_expression);
  }

  [[nodiscard]] constexpr decltype(auto) operator()() const

Comment thread include/deferred/switch.hpp Outdated
}

/// @brief Returns the result of the body expression.
[[nodiscard]] constexpr decltype(auto) operator()() const

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The case_expression class is missing a non-const operator(). This prevents the use of mutable lambdas as the body of a case in a switch_ expression.

  /// @brief Returns the result of the body expression.
  [[nodiscard]] constexpr decltype(auto) operator()()
  {
    return evaluate(m_body);
  }

  /// @brief Returns the result of the body expression.
  [[nodiscard]] constexpr decltype(auto) operator()() const

Comment thread include/deferred/switch.hpp Outdated
Comment thread include/deferred/switch.hpp Outdated
Comment thread include/deferred/while.hpp Outdated
Comment thread include/deferred/switch.hpp Outdated
@ipapadop
ipapadop marked this pull request as ready for review May 25, 2026 18:04
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@ipapadop

Copy link
Copy Markdown
Owner Author

@gemini-code-assist please review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors deferred conditional expressions to support chaining with else_if and else_, and introduces a new homogenized_type trait to support heterogeneous return types via std::variant. It also refactors switch_expression and while_expression to store subexpressions directly instead of using tuples. The reviewer feedback highlights potential use-after-move bugs in switch_expression::choose_case due to improper forwarding, and notes that several visit methods across conditional_expression, switch_expression, and while_expression call the visitor directly on subexpressions instead of recursively propagating the visitation via .visit().

Comment thread include/deferred/switch.hpp Outdated
Comment thread include/deferred/switch.hpp Outdated
Comment thread include/deferred/conditional.hpp
Comment thread include/deferred/switch.hpp
Comment thread include/deferred/switch.hpp
Comment thread include/deferred/switch.hpp
Comment thread include/deferred/while.hpp
@ipapadop
ipapadop merged commit 385142d into main May 25, 2026
5 checks passed
@ipapadop
ipapadop deleted the if-condition branch May 25, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant