Skip to content
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 fmt::formatted_size() with compiled format (FMT_COMPILE) as argument #2161

Merged

Conversation

alexezeder
Copy link
Contributor

@alexezeder alexezeder commented Mar 1, 2021

Resolves #2141.

All useful info can be found in the issue, including code examples.

One small detail for this PR - fmt::formatted_size() in compile.h becomes templatized too to eliminate one unclear case - https://godbolt.org/z/Yxnccj. In few words, the implementation of fmt::formatted_size() for runtime API (non-compiled format string) changes depending on whether we include fmt/compile.h or not. You can see it in assembly for the formatted_size, it uses counting_iterator, which is used in the overload from compile.h:

fmt/include/fmt/compile.h

Lines 837 to 840 in 835b910

template <typename CompiledFormat, typename... Args>
size_t formatted_size(const CompiledFormat& cf, const Args&... args) {
return format_to(detail::counting_iterator(), cf, args...).count();
}

but not in the overload from core.h:

fmt/include/fmt/core.h

Lines 1864 to 1870 in 835b910

template <typename... Args>
inline size_t formatted_size(string_view format_str, Args&&... args) {
const auto& vargs = fmt::make_args_checked<Args...>(format_str, args...);
detail::counting_buffer<> buf;
detail::vformat_to(buf, format_str, vargs);
return buf.count();
}

Maybe it's okay, but IMHO in this case, that behavior should be documented, right?

@vitaut
Copy link
Contributor

vitaut commented Mar 7, 2021

the implementation of fmt::formatted_size() for runtime API (non-compiled format string) changes depending on whether we include fmt/compile.h or not.

Good catch, formatted_size in fmt/compile.h was previously underconstrained.

@vitaut vitaut merged commit 6a9016e into fmtlib:master Mar 7, 2021
@alexezeder alexezeder deleted the fix/formatted_size_with_FMT_COMPILE branch March 8, 2021 00:57
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.

formatted_size() is broken for modern compile-time API
2 participants