-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Enable inheriting from formatter<std::string_view> #4055
Enable inheriting from formatter<std::string_view> #4055
Conversation
fd8e243
to
a0d85e6
Compare
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!
include/fmt/format.h
Outdated
// Fixes issue #4036 | ||
template <typename Char> | ||
struct formatter<detail::std_string_view<Char>, Char> | ||
: public formatter<basic_string_view<Char>, Char> { | ||
auto format(const detail::std_string_view<Char>& value, | ||
format_context& ctx) const -> decltype(ctx.out()) { | ||
using base = formatter<basic_string_view<Char>, Char>; | ||
return base::format(value, ctx); | ||
} | ||
}; |
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.
Looks like I forgot to actually comment what the requested change was =). Let's apply the change to FMT_FORMAT_AS
rather than just the formatter
specialization for string_view
.
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.
No problem, have updated review (haven't squashed yet so you can see diffs), with what I believe you're saying. Just check the types of the params and this is what you meant etc.
a0d85e6
to
0b932de
Compare
include/fmt/format.h
Outdated
template <typename Char> \ | ||
struct formatter<Type, Char> : formatter<Base, Char> { \ | ||
template <typename FormatContext> \ | ||
auto format(Type value, FormatContext&& ctx) const \ |
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.
We either have to pass Type
as a value like here, or turn this parameter into a forwarding reference too otherwise we get const errors. I opted for a value as the types for this are all small (<=16bytes)
Ditto on const issues with passing const format_context&
or format_context&
so have used a forwarding ref for that too (if there's a better way for this one let me know)
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.
FormatContext
should always be passed by a normal reference (&
).
Allows (for example) types convertible to std::string_view to inherit from the fmt::formatter<fmt::string_view> to work etc.
0980555
to
0c06809
Compare
Merged, thanks! |
- Update from version 11.0.1 to 11.0.2 - Update of rootfile - Changelog 11.0.2 - Fixed compatibility with non-POSIX systems (fmtlib/fmt#4054, fmtlib/fmt#4060). - Fixed performance regressions when using `std::back_insert_iterator` with `fmt::format_to` (fmtlib/fmt#4070). - Fixed handling of `std::generator` and move-only iterators (fmtlib/fmt#4053, fmtlib/fmt#4057). Thanks @Arghnews. - Made `formatter<std::string_view>::parse` work with types convertible to `std::string_view` (fmtlib/fmt#4036, fmtlib/fmt#4055). Thanks @Arghnews. - Made `volatile void*` formattable (fmtlib/fmt#4049, fmtlib/fmt#4056). Thanks @Arghnews. - Made `Glib::ustring` not be confused with `std::string` (fmtlib/fmt#4052). - Made `fmt::context` iterator compatible with STL algorithms that rely on iterator category (fmtlib/fmt#4079). Signed-off-by: Adolf Belka <[email protected]> Signed-off-by: Michael Tremer <[email protected]>
Fixes #4036
This formatter specialization with
base::format
means a class implicitly convertible tostd::string_view
will now be converted by this format function before being passed to thefmt::string_view
format function.This wouldn't work previously as the compiler may only perform one implicit conversion, and we need 2 here (from our type, to
std::string_view
, tofmt::string_view
).