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

Formatting of custom types is broken #650

Closed
alabuzhev opened this issue Feb 14, 2018 · 4 comments
Closed

Formatting of custom types is broken #650

alabuzhev opened this issue Feb 14, 2018 · 4 comments

Comments

@alabuzhev
Copy link
Contributor

If a custom type only provides streaming to std::wostream, this code in ostream.h breaks the compilation, as it only tests std::ostream:

struct test_stream : std::ostream {
 private:
  struct null;
  // Hide all operator<< from std::ostream.
  void operator<<(null);
};

// Disable conversion to int if T has an overloaded operator<< which is a free
// function (not a member of std::ostream).
template <typename T>
class convert_to_int<T, true> {
 private:
  template <typename U>
  static decltype(
    std::declval<test_stream&>() << std::declval<U>(), std::true_type())
      test(int);

  template <typename>
  static std::false_type test(...);

 public:
  static const bool value = !decltype(test<T>(0))::value;
};

fmt supports wide streams in all other places and this worked fine before and works now if the code above is commented.

@alabuzhev
Copy link
Contributor Author

Moreover, this logic is questionable in the first place, since it shadows the primary convert_to_int template instead of extending it.

. The comment says:

Disable conversion to int if T has an overloaded operator<<

But what what the code actually does is:

Enable conversion to int if T does not have an overloaded operator<<

Which means that struct foo {}; is literally convertible to int from the library's perspective.

@vitaut
Copy link
Contributor

vitaut commented Feb 17, 2018

Good catch, fixed in 91721ca.

Moreover, this logic is questionable in the first place, since it shadows the primary convert_to_int template instead of extending it.

I'm not a big fan of this approach either. Suggestions on how to improve it are welcome.

Which means that struct foo {}; is literally convertible to int from the library's perspective.

Not really, because of an enum check in

template <typename C, typename T>
inline typename std::enable_if<
    convert_to_int<T, typename C::char_type>::value && std::is_enum<T>::value,
    typed_value<C, int_type>>::type
  make_value(const T &val) { return static_cast<int>(val); }

All this is basically needed to handle enums properly because they (1) can be implicitly convertible to int and (2) can have overloaded operator<<.

@vitaut vitaut closed this as completed Feb 17, 2018
@alabuzhev
Copy link
Contributor Author

Thank you!

All this is basically needed to handle enums properly

Shouldn't is_enum check go first then to not even try to instantiate all that magic for custom types?
This might also slightly improve the compilation time.

@vitaut
Copy link
Contributor

vitaut commented Feb 18, 2018

Shouldn't is_enum check go first then to not even try to instantiate all that magic for custom types?

Good idea. Done in 0ee4273.

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