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

Including fmt/ranges.h and magic_enum/magic_enum_format.hpp fails to compile #4168

Closed
edelmanjm opened this issue Sep 19, 2024 · 8 comments
Closed
Labels

Comments

@edelmanjm
Copy link

(Copying from Neargye/magic_enum#379 for visibility; I suspect it's an issue with magic_enum's templating, but wanted to ensure it's not an issue with fmt.)

magic_enum provides the following template specialization to enable serialization of enums:

template <typename E>
struct fmt::formatter<E, std::enable_if_t<std::is_enum_v<std::decay_t<E>> && magic_enum::customize::enum_format_enabled<E>(), char>> : fmt::formatter<std::string_view> {
  auto format(E e, format_context& ctx) const {
    static_assert(std::is_same_v<char, string_view::value_type>, "formatter requires string_view::value_type type same as char.");
    using D = std::decay_t<E>;

    if constexpr (magic_enum::detail::supported<D>::value) {
      if constexpr (magic_enum::detail::subtype_v<D> == magic_enum::detail::enum_subtype::flags) {
        if (const auto name = magic_enum::enum_flags_name<D>(e); !name.empty()) {
          return formatter<std::string_view, char>::format(std::string_view{name.data(), name.size()}, ctx);
        }
      } else {
        if (const auto name = magic_enum::enum_name<D>(e); !name.empty()) {
          return formatter<std::string_view, char>::format(std::string_view{name.data(), name.size()}, ctx);
        }
      }
    }
    return formatter<std::string_view, char>::format(std::to_string(magic_enum::enum_integer<D>(e)), ctx);
  }
};

Unfortunately, while this specialization works fine on its own, including ranges.h breaks this specalization with a compile error.

  • On Godbolt this appears to be a magic_enum requires enum implementation and valid max and min. (even though the default max and min should be sufficient, and it compiles fine as long as fmt/ranges.h isn't included)
  • On my local compiler with a more complex example, I get an ambiguous template instantiation error. I suspect this is closer to the actual error:
ranges.h:674:47: error: ambiguous template instantiation for ‘struct fmt::v11::formatter<std::byte, char, void>’
  674 |   formatter<remove_cvref_t<value_type>, Char> value_formatter_;
      |                                               ^~~~~~~~~~~~~~~~
format.h:3975:8: note: candidates are: ‘template<class T, class Char> struct fmt::v11::formatter<T, Char, typename std::enable_if<fmt::v11::detail::has_format_as<T>::value, void>::type> [with T = std::byte; Char = char]’
 3975 | struct formatter<T, Char, enable_if_t<detail::has_format_as<T>::value>>
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
magic_enum_format.hpp:86:13: note:                 ‘template<class E> struct fmt::v11::formatter<E, typename std::enable_if<(is_enum_v<typename std::decay<_Tp>::type> && enum_format_enabled<E>()), char>::type> [with E = std::byte]’
   86 | struct fmt::formatter<E, std::enable_if_t<std::is_enum_v<std::decay_t<E>> && magic_enum::customize::enum_format_enabled<E>(), char>> : fmt::formatter<std::string_view> {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I suspected this may have been a issue like #4058. However, adding a specialization to opt it out of range formatting didn't work either, and I got the same compiler error.

#include <fmt/ranges.h>

...

template <typename E>
struct fmt::is_range<E, std::enable_if_t<std::is_enum_v<std::decay_t<E>> && magic_enum::customize::enum_format_enabled<E>(), char>> {
  static constexpr const bool value = false;
};

Any guesses as to what we might be doing wrong?

@vitaut
Copy link
Contributor

vitaut commented Sep 20, 2024

Including fmt/ranges.h is a red herring. As the error says, there is an ambiguity between magic_enum's formatter and the standard formatter(s) provided the {fmt} library. In general having such weakly constrained specializations is a bad idea, one should only specialize formatters for types that they own.

@edelmanjm
Copy link
Author

edelmanjm commented Sep 20, 2024

Understood, thank you. I understand that this is generally not considered best practice; however, there are several hundred enums I'd like to be able to print using {fmt}, and creating an individual format() definition for each is less than desirable. Is there a way to scope the specialization to all enums in a certain namespace or set of namespaces? Or is there some other alternative specialization that won't collide with the specializations in fmt?

For context: I'm trying to migrate from v9.1.0 to v.11.0.2, but the removal of the default enum specialization makes this somewhat more difficult. Adding a format_as() for each enum would mean hundreds and hundreds of lines of duplicated code across the codebase, since they'd all just be calling magic_enum.

@vitaut
Copy link
Contributor

vitaut commented Sep 21, 2024

You can make all enums in your namespace formattable with a generic format_as (#3088 (comment)).

@edelmanjm
Copy link
Author

Thanks, but unfortunately I cannot figure out how to get the template args for the generic format_as to work. Here's what I have at the moment. Does the template itself need to check whether the type parameter is in the namespace? If so, how?

@vitaut
Copy link
Contributor

vitaut commented Sep 24, 2024

Your usage of enable_if is incorrect. Here's the corrected version: https://godbolt.org/z/czq9x3fqP. You don't need to (and can't) explicitly check the namespace in SFINAE, this will be taken care of by ADL.

@edelmanjm
Copy link
Author

Much appreciated, thank you!

@edelmanjm
Copy link
Author

For multiple or nested namespaces: is the recommended method of reusing the format_as() function just to define it in a separate header, then include it within each namespace?

@vitaut
Copy link
Contributor

vitaut commented Sep 25, 2024

I would recommend injecting names in the desired namespace with using declarations (https://en.cppreference.com/w/cpp/language/using_declaration) instead of having an #include in a namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants