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

to_string_view interferes with dynamic_format_arg_store #2180

Closed
BillyDonahue opened this issue Mar 15, 2021 · 3 comments
Closed

to_string_view interferes with dynamic_format_arg_store #2180

BillyDonahue opened this issue Mar 15, 2021 · 3 comments

Comments

@BillyDonahue
Copy link
Contributor

BillyDonahue commented Mar 15, 2021

The presence of a to_string_view(T) function (visible by ADL as usual) breaks dynamic_format_arg_store.push_back(T).

https://gcc.godbolt.org/z/j1xdje

The problem is probably just the use of detail::is_string<T> in the private stored_type<T> metafunction.
Anything with a to_string_view(T) will have a detail::is_string<T>::value == true.

https://github.com/fmtlib/fmt/blob/master/include/fmt/args.h#L97-L99

#include <fmt/args.h>
#include <fmt/format.h>

#include <iostream>
#include <string>
#include <string_view>

struct UserType {
  std::string name;
#ifdef HAS_TO_STRING_VIEW
  friend std::string_view to_string_view(const UserType& ut) { return ut.name; }
#endif
};

template <>
class fmt::formatter<UserType> : fmt::formatter<std::string_view> {
    using Base = fmt::formatter<std::string_view>;
public:
    using Base::parse;
    template <typename FC>
    auto format(const UserType& ut, FC&& fc) {
        return Base::format(ut.name,fc);
    }
};

void print(const std::string& s) { std::cout << s << std::endl; }

int main() {
  UserType ut{"Alice"};
  print(format(FMT_STRING("sanity check: {}"), ut));  // works
  {
    fmt::dynamic_format_arg_store<fmt::format_context> args;
    args.push_back(fmt::arg("userRef", ut));  // std::string(ut) fails
    print(vformat(FMT_STRING("named: {userRef}"), args));
  }
  {
    fmt::dynamic_format_arg_store<fmt::format_context> args;
    args.push_back(fmt::arg("userRef", std::cref(ut)));  // std::string(ut) fails
    print(vformat(FMT_STRING("named,cref: {userRef}"), args));
  }
}

When we build with -DHAS_TO_STRING_VIEW, we get a cascade of failure:
The salient part is here:

/opt/compiler-explorer/libs/fmt/trunk/include/fmt/args.h: In instantiation of 'constexpr fmt::v7::detail::dynamic_arg_list::typed_node<T>::typed_node(const Arg&) [with Arg = UserType; T = std::__cxx11::basic_string<char>]':
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/args.h:54:52:   required from 'const T& fmt::v7::detail::dynamic_arg_list::push(const Arg&) [with T = std::__cxx11::basic_string<char>; Arg = UserType]'
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/args.h:201:64:   required from 'void fmt::v7::dynamic_format_arg_store<Context>::push_back(const fmt::v7::detail::named_arg<typename Context::char_type, T>&) [with T = UserType; Context = fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<char>, char>; typename Context::char_type = char]'
<source>:33:43:   required from here
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/args.h:43:57: error: no matching function for call to 'std::__cxx11::basic_string<char>::basic_string(const UserType&)'
   43 |     FMT_CONSTEXPR typed_node(const Arg& arg) : value(arg) {}
      |                                                         ^

@vitaut
Copy link
Contributor

vitaut commented Mar 15, 2021

Thanks for reporting. We should definitely fix this but is there any particular reason you define to_string_view for your struct?

@BillyDonahue
Copy link
Contributor Author

IRL it's a legacy string_view-like type and we used the to_string_view overload to cheaply make it formattable without having to write a whole fmt::formatter. The formatter specialization in the repro here is just to show that the code compiles without the -DHAS_TO_STRING_VIEW.

When I upgraded from libfmt-6.1.1 to -7.1.3, I started using FMT_COMPILE and dynamic_format_arg_store and hitting this issue and #2181.

I've since written a fmt::formatter specialization for the type, to work around these issues.

@vitaut
Copy link
Contributor

vitaut commented Mar 18, 2021

Fixed in 6ae402f.

@vitaut vitaut closed this as completed Mar 18, 2021
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

No branches or pull requests

2 participants