Add support for Flag enums

[mwittgen]

With the major overhaul of enums in nanobind 2.0, the Flag enum type could be easily supported. Some existing pybind11 applications rely on enums supporting bitwise operations with the result being an enum and not an int.

[chrreisinger]

Adding Flag support would be highly appreciated, since we use flags in our application and the new enum behaviour is blocking us from upgrading to 2.0.

[wjakob]

@chrreisinger

Adding Flag support would be highly appreciated, since we use flags in our application and the new enum behaviour is blocking us from upgrading to 2.0.

Could you tell me which behavior changed? The new enums should mostly behave like the old ones.

[chrreisinger]

@chrreisinger

Adding Flag support would be highly appreciated, since we use flags in our application and the new enum behaviour is blocking us from upgrading to 2.0.

Could you tell me which behavior changed? The new enums should mostly behave like the old ones.

Sure.

A simplified version of the code is:

enum class Foo {
    A = 1,
    B = 2,
    C = 4,
    D = 8,
    E = 16,
    F = 32,
    ...
};

nb::enum_<pynf::Foo>(m, "Foo", nb::is_arithmetic())
        .value("A", pynf::Foo::A)
        .value("B", pynf::Foo::B)
        .value("C", pynf::Foo::C)
        .value("D", pynf::Foo::D)
        .value("E", pynf::Foo::E)
        .def("__bool__", [](const pynf::Foo val) { return pynf::to_underlying(val) != 0; })
        .def("__or__",
             [](const pynf::Foo a, const pynf::Foo b) {
                 return pynf::Foo(pynf::to_underlying(a) | pynf::to_underlying(b));
             })
        .def("__and__", [](const pynf::Foo a, const pynf::Foo b) {
            return pynf::Foo(pynf::to_underlying(a) & pynf::to_underlying(b));
        });

And at runtime we get an error that a combination of the flags is not valid.

| ValueError: 31095 is not a valid Foo.

[wjakob]

Aha! Yes, that makes sense. The fact that it worked previously is effectively undefined behavior. We expect that the user returns a valid/bound enum but do not check if that is actually the case.

[wjakob]

What's the status of this PR? Do you still plan to make changes? (it's in draft mode) Would you like me to review it?

[mwittgen]

There are some more unresolved issues, for example Flag.A | Flag.B can't be passed to a C++ function void func(enum_type e) Looks like this needs some type casters for enum.Flag

[wjakob]

Yeah, that seems tricky. The existing type caster does a hash table lookup that looks for enumerants. In this case, you'd need to either need to populate the table with all possible combinations (combinatorial blowup) or perform some kind of integer conversion. I assume that the same issue would appear on the way back. The special case to support flag enums in this way might add a computational cost to the default enums (which I would not want). I don't have any suggestions, unfortunately.

[oremanj]

It seems to me that it would work fine to fall back from the map lookups to a slightly slower alternate, either enum_instance.value for Py->C++ conversions or EnumClass(number) for C++->Py. This fallback could be done only for flag enums in order to avoid slowing down failing conversions of regular enums during overload resolution. Flag generates the objects representing flag combinations as they're requested, but they do still get "singletonized" / cached in the member2value_map, so we could choose to update our maps as well; then the slow path only needs to be taken once per combination. There are some other possible optimizations as well, such as caching the mask of which flags are defined, and preemptively rejecting lookups outside of that mask.

[mwittgen]

Fixed the 3.11+ compilation failures with stable ABI enabled.

[wjakob]

It would be helpful to have feedback from people who are invested into this -- @skallweitNV, @keithlostracco.

Does it look okay to you as well @oremanj? From what I can see, the modified PR now adopts your suggestion.

[skallweitNV]

Question: Does it make sense to base nanobind's enum types on Enum and Flag in the first place? Wouldn't IntEnum and IntFlag (for enums with nb::is_arithmetic) be the more appropriate types? After all, we are binding C++ enums which are always of some integer type.

[wjakob]

For non-arithmetic enums (e.g. C++11-style enum classes), Enum is the closest match in Python-land. For arithmetic ones, we use IntEnum. That's how the implementation works now, and I think that part makes sense.

Is your point mainly about flags?

[keithlostracco]

This appears to be exactly the functionality I was looking for.

[skallweitNV]

For non-arithmetic enums (e.g. C++11-style enum classes), Enum is the closest match in Python-land. For arithmetic ones, we use IntEnum. That's how the implementation works now, and I think that part makes sense.

True, for class enums the Enum is the closest match.

Is your point mainly about flags?

Yes, I need support for flags. Supporting the full matrix for is_arithmetic and is_flag seems like the perfect solution.