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.88k stars 434 forks source link

customize::enum_name<E>() doesn't invalidate enum items #319

Closed svew closed 10 months ago

svew commented 11 months ago

My team has a long history of tacking "Count" to the end of enums as an easy way to check the length of the enum. I wanted to specialize magic_enum to disregard the last "Count" item if it exists using the customize feature. I'm running into problems.

Looking at the below code (https://godbolt.org/z/94arehro9):

enum class Days
{
    Monday,
    Tuesday,
    Wednesday,
    Thursday,
    Friday,
    Saturday,
    Sunday,
    Count
};

namespace magic_enum::customize
{
    template <typename E>
        requires std::is_enum_v<E> // To create preference for this method
    constexpr customize_t enum_name(E e) noexcept {
        return invalid_tag; // Invalidate all enum items
    }
}

// Works!
static_assert(magic_enum::customize::enum_name<Days>(Days::Monday).first
           == magic_enum::customize::detail::customize_tag::invalid_tag);

// Doesn't...
static_assert(false == magic_enum::detail::is_valid<Days, Days::Monday>());

// Doesn't...
static_assert(0 == magic_enum::enum_count<Days>());

I would expect that enum_count<Days>() would return 0, but instead it doesn't seem to use the customization, even though it seems like is_valid<Days, Days::Monday>() absolutely should?

template <typename E, auto V>
constexpr bool is_valid() noexcept {
  constexpr E v = static_cast<E>(V);
  [[maybe_unused]] constexpr auto custom = customize::enum_name<E>(v); // This value is invalid_tag, per first static_assert
  static_assert(std::is_same_v<std::decay_t<decltype(custom)>, customize::customize_t>, "magic_enum::customize requires customize_t type.");
  if constexpr (custom.first == customize::detail::customize_tag::custom_tag) {
    constexpr auto name = custom.second;
    static_assert(!name.empty(), "magic_enum::customize requires not empty string.");
    return name.size() != 0;
  } else if constexpr (custom.first == customize::detail::customize_tag::default_tag) {
    return n<v>().size_ != 0;
  } else {
    return false; // ... so it should return false?
  }
}
Neargye commented 10 months ago

Hi, It seems to me that the problem here is that you are doing redefinition and not specialization

https://godbolt.org/z/dd1ePh5Yo

svew commented 10 months ago

I see, maybe I misunderstood template matching relating to concepts...

Like I mentioned earlier, I was hoping to apply this customization to all of my team's enums to check whether the last item is "Count" and disregard it. Would you know of any other way to apply my rules across every enums evaluated so I don't have to identify which of our thousands of enums have "Count" and which don't? A macro switch would probably solve my problem

svew commented 10 months ago

A macro switch like this for example:

namespace magic_enum::customize {

// Default customize.
inline constexpr auto default_tag = customize_t{detail::customize_tag::default_tag};
// Invalid customize.
inline constexpr auto invalid_tag = customize_t{detail::customize_tag::invalid_tag};

#if !defined(MAGIC_ENUM_CUSTOMIZE_OVERRIDE_ENUM_NAME)
  // If need custom names for enum, add specialization enum_name for necessary enum type.
  template <typename E>
  constexpr customize_t enum_name(E) noexcept {
    return default_tag;
  }
#else
  template <typename E>
  constexpr customize_t enum_name(E) noexcept;
#endif

#if !defined(MAGIC_ENUM_CUSTOMIZE_OVERRIDE_ENUM_TYPE_NAME)
  // If need custom type name for enum, add specialization enum_type_name for necessary enum type.
  template <typename E>
  constexpr customize_t enum_type_name() noexcept {
    return default_tag;
  }
#else
  template <typename E>
  constexpr customize_t enum_type_name() noexcept;
#endif

} // namespace magic_enum::customize

Besides this, I'm not sure how else to generically override customize::enum_name()

svew commented 10 months ago

Actually, I'm not sure if just the above will let me achieve what I'm looking for. I was hoping to use the n<auto V>() function to grab the reflected name and check if it contains "Count", but with the customize::enum_name<typename E>(E) interface that already exists, I don't believe I'm able to grab the reflected name of the given enum value. It'd need to be customize::enum_name<typename E, E V>(), which looks like it will work fine with existing uses of customize::enum_name, but may break backwards compatibility. Any thoughts about this? I know n<auto V>() is under detail namespace so probably shouldn't be relied on, but I don't see another way right now.

Another issue is checking whether the given enum value is the last value of the enum. That sort of thing seems like it would require intercepting valid_count() before it's returned back to values() for use. It'd be nice if there were a way to do this, but I don't think it's necessary for us, so if it's too complicated then it's not a concern.

Neargye commented 10 months ago

I’ll think about how to do it in a simpler way

Neargye commented 10 months ago

Unfortunately, I did not find an easy way to add this kind of customization to the library.

Probably the best way for you is fork/patch the library and update the function is_valid

Example

template <typename E, auto V>
constexpr bool is_valid() noexcept {
  constexpr auto name = n<V>();
  return name.size_ > 0 && (string_view{name.str_, name.size_}.compare("Count") != 0);
}