Skip to content

Generic::to_*() enhancements#672

Merged
liuzicheng1987 merged 1 commit into
getml:mainfrom
kone-tlammi:main
May 31, 2026
Merged

Generic::to_*() enhancements#672
liuzicheng1987 merged 1 commit into
getml:mainfrom
kone-tlammi:main

Conversation

@kone-tlammi
Copy link
Copy Markdown
Contributor

@kone-tlammi kone-tlammi commented May 29, 2026

  • The new version avoids extra copies in case the current Generic
    contents do not match the casted-to type.
  • Using std::get_if avoids switch-case generation for all the variant types
    when we are only interested in one or two types.

Copy link
Copy Markdown

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

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 optimizes the conversion helper functions in include/rfl/Generic.hpp by passing the variant value by const reference (const auto&) instead of by value within the std::visit lambdas. The review feedback suggests a further improvement: replacing std::visit with std::get_if across these functions. This change would avoid the compilation and runtime overhead of instantiating generic lambdas for all variant types, leading to cleaner and more efficient code.

Comment thread include/rfl/Generic.hpp Outdated
- The new version avoids extra copies in case the current Generic
contents do not match the casted-to type.
- Using std::get_if avoids switch-case generation for all the variant types
  when we are only interested in one or two types.
@kone-tlammi kone-tlammi changed the title Avoid extra copies in Generic::to_*() Generic::to_*() enhancements May 29, 2026
@liuzicheng1987
Copy link
Copy Markdown
Contributor

@kone-tlammi , thanks for the contribution! I am merging.

@liuzicheng1987 liuzicheng1987 merged commit 92413ca into getml:main May 31, 2026
181 checks passed
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.

2 participants