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

Regression in PR #323 #330

Closed
p0358 opened this issue Jan 1, 2024 · 3 comments
Closed

Regression in PR #323 #330

p0358 opened this issue Jan 1, 2024 · 3 comments
Milestone

Comments

@p0358
Copy link

p0358 commented Jan 1, 2024

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
Copy link
Owner

Neargye commented Jan 1, 2024

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

@Neargye Neargye added this to the v0.9.6 milestone Jan 1, 2024
@Neargye Neargye closed this as completed Feb 1, 2024
@p0358
Copy link
Author

p0358 commented May 30, 2024

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
Copy link
Owner

Neargye commented May 30, 2024

yes, might be worth adding a link to limitations

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