Neargye / magic_enum

Static reflection for enums (to string, from string, iteration) for modern C++, work with any enum type without any macro or boilerplate code
MIT License
4.76k stars 422 forks source link

Regression in PR #323 #330

Closed p0358 closed 7 months ago

p0358 commented 8 months ago

After #323 it seems enum class doesn't work anymore. I have en example enum

enum class MasterserverMessageType
{
    M2H_ACCEPT = 1000,
    M2H_HOST_GAME_INIT = 1002,
    L2H_HOST_PLAYER_INIT = 1003,
    M2H_GAME_LEAVE = 1007,
};

and example invokation

auto test = magic_enum::enum_name((MasterserverMessageType)1003);

fails with:

thirdparty\magic_enum\include\magic_enum\magic_enum.hpp(1291,17): error C2338: static_assert failed: 'magic_enum requires enum implementation and valid max and min.' (compiling source file ..\Test.cpp)
Test.cpp(238,29): message : see reference to function template instantiation 'std::basic_string_view<char,std::char_traits<char>> magic_enum::enum_name<MasterserverMessageType,magic_enum::detail::enum_subtype::common>(E) noexcept' being compiled
        with
        [
            E=MasterserverMessageType
        ]

It seems to point at this static_assert failing:

// Returns name from enum value.
// If enum value does not have name or value out of range, returns empty string.
template <typename E, detail::enum_subtype S = detail::subtype_v<E>>
[[nodiscard]] constexpr auto enum_name(E value) noexcept -> detail::enable_if_t<E, string_view> {
  using D = std::decay_t<E>;
  static_assert(detail::is_reflected_v<D, S>, "magic_enum requires enum implementation and valid max and min.");

  if (const auto i = enum_index<D, S>(value)) {
    return detail::names_v<D, S>[*i];
  }
  return {};
}

Worth pointing out that before updating magic_enum, the code worked perfectly fine, so this really points at the checks being wrong. Compiler is MSVC from Visual Studio 2022 (latest v17.7.7)

Neargye commented 8 months ago

This assert was added to give an error if there are no enum values ​​reflected. In your example, the test value will be empty.

enum class MasterserverMessageType
{
    M2H_ACCEPT = 1000,
    M2H_HOST_GAME_INIT = 1002,
    L2H_HOST_PLAYER_INIT = 1003,
    M2H_GAME_LEAVE = 1007,
};
auto test = magic_enum::enum_name((MasterserverMessageType)1003); // empty string
std::cout << test << std::endl;  // output = ''

You can add the old behavior defined macro #define MAGIC_ENUM_NO_CHECK_REFLECTED_ENUM

p0358 commented 3 months ago

So coming back to it, turns out I basically had to specify something like this

template <>
struct magic_enum::customize::enum_range<MasterserverMessageType> {
    static constexpr int min = (const int)MasterserverMessageType::M2H_ACCEPT;
    static constexpr int max = (const int)MasterserverMessageType::S2L_SECURITY;
    // (max - min) must be less than UINT16_MAX.
};

which was mentioned in the doc/limitations.md file that I for some reason completely missed...

I guess this also would mean that before this commit that has the check, the logs I had probably didn't work ever to begin with and I just never noticed until the error came? Anyway I help my comment helps someone in the future or something.

@Neargye my small suggestion would be to try leading the user from the error message somehow towards how they can fix it or what they could otherwise do to alleviate the issue. If not in the error itself, then perhaps where it's defined some link to https://github.com/Neargye/magic_enum/blob/master/doc/limitations.md#enum-range or something like that? Could help someone in the future too...

Neargye commented 3 months ago

yes, might be worth adding a link to limitations