-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix fallback to runtime API from compile-time API #2143
Fix fallback to runtime API from compile-time API #2143
Conversation
…PI define, add test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Looks good but I don't think we should provide a way to disable the fallback.
include/fmt/compile.h
Outdated
constexpr auto compiled = detail::compile<Args...>(S()); | ||
#ifdef __cpp_if_constexpr | ||
if constexpr (std::is_same<remove_cvref_t<decltype(compiled)>, | ||
detail::unknown_format>()) { | ||
auto it = | ||
format_to(detail::truncating_iterator<OutputIt>(out, n), | ||
static_cast<basic_string_view<typename S::char_type>>(S()), | ||
std::forward<Args>(args)...); | ||
return {it.base(), it.count()}; | ||
} else { | ||
auto it = format_to(detail::truncating_iterator<OutputIt>(out, n), compiled, | ||
std::forward<Args>(args)...); | ||
return {it.base(), it.count()}; | ||
} | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace all of these by forwarding S
to format_to
call below which will handle fallback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can. It will create one more template instantiation, but it shouldn't be tough.
So, it's done.
…e API instead of compiling it inside format_to_n(), to eliminate code duplication
Thank you! |
Right now, when {fmt} fails to prepare compiled format for format string (using
FMT_COMPILE
or_cf
) it also fails to report about that clearly. Here is an example - https://godbolt.org/z/G1ETdc.There were some attempts to make the error a bit clearer, but as it turned out {fmt} should fall back to the runtime API in those cases. This PR adds that fallback.
Some points for the PR: