Add missing Allocator template argument for basic_memory_buffer in fo… #2300
Conversation
include/fmt/format.h
Outdated
| typename Char = enable_if_t<detail::is_string<S>::value, char_t<S>>> | ||
| inline auto format_to(basic_memory_buffer<Char, SIZE>& buf, const S& format_str, | ||
| typename Char = enable_if_t<detail::is_string<S>::value, char_t<S>>, | ||
| typename Allocator = std::allocator<Char>> |
There was a problem hiding this comment.
I think that Allocator shouldn't be defaulted because it's deduced from the argument. Otherwise looks good.
There was a problem hiding this comment.
Good point, isn't that true for all of basic_memory_buffer's template arguments here?
There was a problem hiding this comment.
Never mind, the defaults means I can pass {} and get the defaulted values. I think that means I can remove the default SIZE then.
There was a problem hiding this comment.
It's true for SIZE (could you remove the default while at it?) but not for Char which is used for SFINAE.
There was a problem hiding this comment.
Oh, but it must be an lvalue. Sorry for the stream of thought here.
There was a problem hiding this comment.
I pushed another version that removes the default template argument for vformat_to. Let me know if you want me to limit the change to just format_to.
There was a problem hiding this comment.
Please limit the changes to format_to.
There was a problem hiding this comment.
How do you see the default argument to Char being used?
I guess something like:
format_to<std::basic_string<wchar_t>>(...)There was a problem hiding this comment.
But it's still deduced from buf, right? 🤔
There was a problem hiding this comment.
Updated to limit the change to format_to.
voxmea
force-pushed
the
master
branch
6 times, most recently
from
cedfe6e
to
967c394
Compare
include/fmt/format.h
Outdated
| template <typename S, typename... Args, typename Char, size_t SIZE, | ||
| typename Allocator, FMT_ENABLE_IF(detail::is_string<S>::value)> | ||
| inline auto format_to(basic_memory_buffer<Char, SIZE, Allocator>& buf, | ||
| const S& format_str, | ||
| Args&&... args) -> |
There was a problem hiding this comment.
Does clang-format still break this onto the next line?
There was a problem hiding this comment.
Oops, thanks, this time run through clang-format.
…rmat_to Remove deduced default template arguments in format_to and moves the SFINAE check to a non-deduced template parameter.
|
Thank you |
…rmat_to