8000
Skip to content

Fix fmt::formatted_size() with compiled format (FMT_COMPILE) as argument#2161

Merged
vitaut merged 1 commit intofmtlib:masterfrom
alexezeder:fix/formatted_size_with_FMT_COMPILE
Mar 7, 2021
Merged

Fix fmt::formatted_size() with compiled format (FMT_COMPILE) as argument#2161
vitaut merged 1 commit intofmtlib:masterfrom
alexezeder:fix/formatted_size_with_FMT_COMPILE

Conversation

@alexezeder
Copy link
Copy Markdown
Contributor
@alexezeder alexezeder commented Mar 1, 2021 •
edited
Loading

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
Copy Markdown
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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

formatted_size() is broken for modern compile-time API

2 participants

@alexezeder @vitaut
0