Working as intended? Named combined bitflags cannot be casted to

[JWCS]

I believe this is related to #314 and #321, so my apologies if it is. But, from the docs and those comments, it almost seemed like the below use case should work; but it fails for the BOTH case. This is after I considered your casting comments in 314 about enum vs enum_flags, and accounted for both input formats, to try both in what seemed the least restrictive way. I also read through the limitations... so I am very confused.
Working off of head (0.9.6):

#include <magic_enum_flags.hpp>
#include <iostream>
#include <string>

enum class FlowDirection : unsigned {
    NONE  = 0,
    CCW   = 1,
    CW    = 2,
    BOTH  = 3,
};
template <>
struct magic_enum::customize::enum_range<FlowDirection> {
  static constexpr bool is_flags = true;
};
inline static FlowDirection FlowDirection_read(
    std::string const& str, FlowDirection const& def = FlowDirection::NONE)
{
  return magic_enum::enum_flags_cast<FlowDirection>(str, magic_enum::case_insensitive).value_or(
      magic_enum::enum_cast<FlowDirection>(str, magic_enum::case_insensitive).value_or(def));
}

static void test(std::string const& str){
    FlowDirection flow = FlowDirection_read(str);
    std::cout << str << " ("
      << magic_enum::enum_name(flow) << "/"
      << magic_enum::enum_flags_name(flow) << "): "
      << magic_enum::enum_integer(flow) << std::endl;
}

int main(){
    test("CCW");
    test("ccw");
    test("cw");
    test("BOTH");
    test("NONE");
    test("NONE|CCW");
    test("CCW|CW");
    return 0;
}

Which results in incorrect values definitely for BOTH, and a curious non-parsing (?) for "NONE|CCW"

> g++ -std=gnu++17 demo.cpp && ./a.out
CCW (CCW/CCW): 1    # OK
ccw (CCW/CCW): 1    # OK
cw (CW/CW): 2       # OK
BOTH (/): 0         # Uhhhhh help?? I have no clue what I'm doing wrong, to not get this.
NONE (/): 0         # This is interesting; the value is correct, but the name is missing; which might be 321
NONE|CCW (/): 0     # Uh, I don't think I need this, but, is this a bug?
CCW|CW (/CCW|CW): 3 # OK

[Neargye]

BOTH (/): 0 - #314 (comment). BOTH is not true flag, it is a "flag or"

NONE (/): 0 - #321

NONE|CCW (/): 0 - will check, looks like bug, expected 1

[JWCS]

With respect to the first one, BOTH, the reason for my confusion is that the read() function attempts both (in order) enum_flags_cast and then enum_cast.
As per the relevant docs/comment in 314:

enum class TestEnum {
A = 0,
B = 1,
C = 2,
D = 3,
};

enum_name(TestEnum::D) = 'D'; vs enum_flags_name(TestEnum::D) = 'B|C';
enum_flags_cast('A') = nullopt; vs enum_cast('A') = TestEnum::A;
enum_flags_cast('D') = nullopt; vs enum_flags_cast('B|C') = TestEnum::D; vs  enum_cast('D') = TestEnum::D;

I'd expect enum_flags_cast('D' /* BOTH */).value_or( enum_cast('D') to fail on flag-cast D (since not a real flag), but then trying to normal enum cast D to work (enum_cast('D') == TestEnum::D) (and the resulting integer TestEnum::D == TestEnum::B | TestEnum::C).
I tried it like this, such that if the string 'B|C' was passed, that would first get correctly parsed/handled, but if the shorthand 'BOTH' was passed, that would also get correctly handled.
My main question is, is there something I'm missing, that test("BOTH") ->
enum_flags_cast("BOTH") /* fail */ .value_or( enum_cast("BOTH") /* fail */ .value_or("NONE") /* incorrect!? */ ).
Does it have something to do with

template <>
struct magic_enum::customize::enum_range<FlowDirection> {
  static constexpr bool is_flags = true;
};

?

[ZXShady]

The behavior you seeing is indeed related to how is_flags affects the different functions.

1. enum_flags_* functions
   · These functions always treat the enum as flags (regardless of is_flags setting)
   · They only recognize values that are direc powers of 2 this is why BOTH (3) isn't recognized.

2. regular enum functions
   · these are affected by the is_flags customization point.
   · when is_flags is true, they only show power of 2 values (like enum_flags_*)
   · when is_flags is false, they reflect the normal range [-128,127].

In your case:

· With is_flags is true, neither function will recognize BOTH (3) because:
· enum_flags_* never recognizes combined enum member flags (not powers of 2) nor zero.
· regular enum_* skips it because is_flags is true, behaving like enum_flags_.*
· NONE shows as empty because for enums where is_flags is true skips the 0 value.

· "NONE|CCW" parsing to 0 is weird but expected given magic_enum does not reflect 0 value for flags, it thinks NONE is an invalid enum member.

[Neargye]

@JWCS @ZXShady

After checking, yes, "NONE|CCW" parsing to 0 in the current implementation is not a bug. Library doesn't find NONE in the flags list and treats it as an invalid name.

Personally, I don't think zero should be included by default. I would prefer two separate options: flags and flags_with_zero (or flags_and_zero), though this is open for discussion.

[JWCS]

Sorry for the late reply;
I think recognizing zero is crucial for bit flags, because their common semantics are that of a set, and zero is the null or empty set.
It's difficult to do certain set operations without the null set, specifically comparing for empty and accumulating a series of results.

With enum classes, I can't just trivially compare void fn(FlowDirection flags) { if ( flags != 0 ) ..., (though bool(flags) should work), but I would want to compare flags != NONE.
Similarly, if I'm accumulating flags from an input, FlowDirection flags = NONE; for(...) if(...) flags |= ANOTHER_FLAG;. That is, an empty set can be unified / added to more easily.
Additionally, the intersection of a set and a null set should be empty. FlowDirection aligned_flows(FlowDirection d1, FlowDirection d2){ return d1 & d2; } This logic isn't easily implemented today.

If bitflags were just integers, then we trivially still have these operations by & | ==0, but since we've enum-class'ed them separately, both these operations and the data they semantics they operate over must be represented. With magic_enum::bitwise_operators we have the former, but not the later.

Now... this could just were we say "ofc, you're using flags incorrectly, they're not intended to have an empty set, if you really want set semantics, use magic_enum::containers::bitset!!!"
(or optional<T>, but the code gets messier).

---

Thanks for pointing out that adding the is_flags=true modifier was defeating the point of testing/using both the enum cast and enum flags casts.
To recap the 3 "problems" above, first the BOTH and CW|CCW conditions are correctly interpreted with that.
Second, while using flags, using NONE(0) as-is, or combined with other bitflags still is invalid, per the discussion above, is tracked by issue 321,

But as for the last one, it still seems like a bug: without is_flags=true, for regular (not flags) enum opts, NONE|CCW (0|1)-> NONE (0), instead of CCW (1).
I'll leave this issue open to track this last "bug".

[ZXShady]

Sorry for the late reply;
I think recognizing zero is crucial for bit flags, because their common semantics are that of a set, and zero is the null or empty set.
It's difficult to do certain set operations without the null set, specifically comparing for empty and accumulating a series of results.

You are right, I do this myself in my enum reflection library (enchantum) but for magic enum to do it would be an api break but neargye should put more thought into putting 0 therr or making it optional

[Neargye]

@ZXShady
I think I need to add a zero to the flags processing, but it really changes the behavior. It will probably be in the next major version.

[ZXShady]

@ZXShady
I think I need to add a zero to the flags processing, but it really changes the behavior. It will probably be in the next major version.

I can't decide whethee it should be hidden behind a macro and it is enabled globally or per enum basis like enum_range.

template<>
struct enum_range {
      static constexpr bool zero = true;
};