-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix reference_wrapper ambiguity with format_as #4434
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
Conversation
Could you elaborate? |
|
When I attempted the approach of template <typename T>
T& format_as(std::reference_wrapper<T> ref) { return ref.get(); }I got the following error for the test line /mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/base.h: In instantiation of ‘constexpr fmt::v11::detail::value<Context>::value(const T&, fmt::v11::detail::custom_tag) [with T = std::reference_wrapper<int>; typename std::enable_if<(! has_formatter<T, typename Context::char_type>()), int>::type <anonymous> = 0; Context = fmt::v11::context]’:
/mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/base.h:2235:65: required from ‘fmt::v11::detail::value<Context>::value(T&) [with T = std::reference_wrapper<int>; typename std::enable_if<(std::integral_constant<bool, (((((((std::is_class<T>::value || std::is_enum<T>::value) || std::is_union<T>::value) || std::is_array<T>::value) && (! fmt::v11::detail::has_to_string_view<T, void>::value)) && (! fmt::v11::detail::is_named_arg<T>::value)) && (! fmt::v11::detail::use_format_as<T>::value)) && (! fmt::v11::detail::use_format_as_member<typename std::remove_const<T>::type, std::integral_constant<bool, true> >::value))>::value || (!1)), int>::type <anonymous> = 0; Context = fmt::v11::context]’
/mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/format.h:4200:17: required from ‘std::string fmt::v11::format(format_string<T ...>, T&& ...) [with T = {std::reference_wrapper<int>}; std::string = std::__cxx11::basic_string<char>; format_string<T ...> = fstring<std::reference_wrapper<int> >]’
/mnt/c/Users/rifkin/home/projects/open-source/fmt/test/std-test.cc:419:3: required from here
/mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/base.h:2262:45: error: ‘fmt::v11::detail::type_is_unformattable_for<std::reference_wrapper<int>, char> _’ has incomplete type
2262 | type_is_unformattable_for<T, char_type> _;
| ^And similar for Adding the line /mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/base.h: In substitution of ‘template<class T> using fmt::v11::detail::format_as_result = fmt::v11::remove_cvref_t<decltype (format_as(declval<const T&>()))> [with T = std::reference_wrapper<int>]’:
/mnt/c/Users/rifkin/home/projects/open-source/fmt/test/std-test.cc:421:61: required from here
/mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/base.h:1115:38: error: ‘format_as’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
1115 | remove_cvref_t<decltype(format_as(std::declval<const T&>()))>;
| ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/c/Users/rifkin/home/projects/open-source/fmt/include/fmt/std.h:750:4: note: ‘template<class T> T& format_as(std::reference_wrapper<_Tp>)’ declared here, later in the translation unit
750 | T& format_as(std::reference_wrapper<T> ref) { return ref.get(); }
| ^~~~~~~~~I figured it'd be easiest to just update the formatter to check for existing format_as / format_as_member ability |
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.
I suggest using format_as but make it a member (since we can't inject it into the std namespace`).
|
Adding the following to the formatter causes the most bizarre thing to happen static auto format_as(std::reference_wrapper<T> ref) -> T& {
return ref.get();
}Suddenly the to_string here evaluates to int num = 35;
fmt::to_string(std::cref(num));This seems to be because the chosen template <typename T, FMT_ENABLE_IF(!std::is_integral<T>::value &&
!detail::use_format_as<T>::value)>
FMT_NODISCARD auto to_string(const T& value) -> std::string {
auto buffer = memory_buffer();
detail::write<char>(appender(buffer), value);
return {buffer.data(), buffer.size()};
}which writes the int value as a char. Presumably this is because there is no Since any use of member format_as here will depend on the additional constraints for the reference_wrapper formatter specialization this PR adds and there appear to be more involved changes required to get member format_as working, could this PR be merged as-is with the other changes deferred to another PR? |
OK but please move |
|
Thanks, taken care of |
|
Thank you! |
This PR fixes #4424. It was unfortunately not possible to just implement this with
template <typename T> T& format_as(std::reference_wrapper<T>);in the general case so I've just guarded the formatter specialization for format_as.Happy to adjust the changes here if needed!