Generic overloading bitwise functions to manipulate the enum type

Question

Thanks to G. Sliepen, who gave me a lot very meaningful & useful advice on my implementation about overloading bit operators for a special scoped enum.

Now I have modified the code snippet to follow his advice.

For creating generic overloading bitwise functions to manipulate the enum type, I think my implementation can indeed accomplish this goal (code is seen below).

This and this code review use a somewhat complex way to achieve the said goal, which is still hard for me to read and understand (I will try my best to fully understand them later).

Is there any potential problem with my implementation?

Here is my code:

#include <iostream>
#include <stdint.h>
#include <type_traits>

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T operator|(const T& a, const T& b)
{
    using udl_type = typename std::underlying_type<T>::type;
    
    return static_cast<T>(static_cast<udl_type>(a) | static_cast<udl_type>(b));
}

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T& operator|=(T& a, const T& b)
{
    a=(a|b);
    return a;
}

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T operator&(const T& a, const T& b)
{
    using udl_type = typename std::underlying_type<T>::type;

    return static_cast<T>(static_cast<udl_type>(a) & static_cast<udl_type>(b));
}

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T operator&=(T& a, const T& b)
{
    a = a&b;
    return a;
}

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T operator^(const T& a, const T& b)
{
    using udl_type = typename std::underlying_type<T>::type;

    return static_cast<T>(static_cast<udl_type>(a) ^ static_cast<udl_type>(b));
}

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T& operator^=(T& a, const T& b)
{
    a = a ^ b;
    return a;
}

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
T operator~(const T& a)
{
    using udl_type = typename std::underlying_type<T>::type;
    return static_cast<T>(~static_cast<udl_type>(a));
}

class ModuleSwitch
{
public:
    enum class CameraPosition : uint8_t
    {
        FRONT_CAMERA = 1,  //NONE is remove to avoid the user calls enable(ModuleSwitch::NONE);
        MIDDLE_CAMERA = 2,
        BACK_CAMERA = 4,
    };
public:
    ModuleSwitch();
    void enable(const CameraPosition&);
    void disable(const CameraPosition&);
    bool is_enabled(const CameraPosition&) const;
private:
    CameraPosition m_mod_status;
};

//NONE is remove to avoid the user calls enable(ModuleSwitch::NONE);
//Is there any better to way to init m_mod_status?
ModuleSwitch::ModuleSwitch(): m_mod_status(static_cast<CameraPosition>(0)){}; 

void ModuleSwitch::enable(const CameraPosition& pos)
{
    m_mod_status |= pos;
}

void ModuleSwitch::disable(const CameraPosition& pos)
{
    m_mod_status &= ~pos;
}

bool ModuleSwitch::is_enabled(const CameraPosition& pos) const
{
    return static_cast<std::underlying_type<CameraPosition>::type>(m_mod_status & pos)!=0;
}

int main() {
    ModuleSwitch mod_swt;
    using CameraPosition = ModuleSwitch::CameraPosition;
    auto print_camera_status = [](const ModuleSwitch& mod_swt){
        std::cout << "front camera enabled:" << mod_swt.is_enabled(CameraPosition::FRONT_CAMERA) << std::endl;
        std::cout << "middle camera enabled:" << mod_swt.is_enabled(CameraPosition::MIDDLE_CAMERA) << std::endl;;
        std::cout << "back camera enabled:" << mod_swt.is_enabled(CameraPosition::BACK_CAMERA) << std::endl;;
    };

    std::cout <<"#1" << std::endl;
    mod_swt.enable(CameraPosition::FRONT_CAMERA);
    print_camera_status(mod_swt);

    std::cout <<"#2" << std::endl;
    mod_swt.enable(CameraPosition::MIDDLE_CAMERA);
    print_camera_status(mod_swt);

    std::cout <<"#3" << std::endl;
    mod_swt.enable(CameraPosition::BACK_CAMERA);
    print_camera_status(mod_swt);

    std::cout <<"#4" << std::endl;
    mod_swt.disable(CameraPosition::FRONT_CAMERA);
    print_camera_status(mod_swt);

    std::cout <<"#5" << std::endl;
    mod_swt.disable(CameraPosition::MIDDLE_CAMERA);
    print_camera_status(mod_swt);

    std::cout <<"#6" << std::endl;
    mod_swt.disable(CameraPosition::BACK_CAMERA);
    print_camera_status(mod_swt);
    
    return 0;
}

Answer 1

Consider std::to_underlying

You can replace static_cast<udl_type>(a) with std::to_underlying(a), and not need to declare udl_type at all.

This could simplify the code, compared to extracting the type and doing three static_cast operations on the same line.

Perhaps Check that the Result is a Valid enum Value

There is unfortunately no way to do this generically. You could, however, define

template<typename T> constexpr bool is_legal_value(const T& input) noexcept {
    return true;
}

And then override it for each type where twiddling bits could actually produce an illegal value, such as by comparing to the first and last values, or a switch statement. Then, you could add to the bitwise functions a block such as:

    if (!is_legal_value(result)) {
        throw std::logic_error("Bitwise operation yielded illegal enum value.");
    }

This should be optimized out and have zero cost if the code uses the trivial default implementation.

Answer 2

All overloads can be made constexpr

You can make all the overloaded functions constexpr, so they can be used in expressions that need to be evaluated at compile time.

Not every enum is like a bitfield

Your operator overloads work well for CameraPosition, where each value is a power of two, and thus combinations of them make sense. However, this is not true for all enums. It would be nice to be able to select which enums to enable the overloads for. That can be done by creating a templated concept, class or even just a constexpr bool that by default returns false for any type, but can be specialized to return true for specific types. For example:

template<typename T>
inline constexpr bool is_bitfield<T> = false;

template<class T, typename = typename std::enable_if<std::is_enum<T>::value>::type>
requires is_bitfield<T>
T operator|(const T& a, const T& b)
{
    using udl_type = typename std::underlying_type<T>::type;
    
    return static_cast<T>(static_cast<udl_type>(a) | static_cast<udl_type>(b));
}
…
enum Foo {FOO, BAR};
enum Bar {BAZ, QUUX};

template<>
inline constexpr bool is_bitfield<Foo> = true;

auto foobar  = Foo::FOO | Foo::BAR;  // compiles
auto bazquux = Bar::BAZ | Bar::QUUX; // error: no match for ‘operator|’

You also might want to consider whether you want to allow your operators to work on enums whose underlying type is a signed integer, even if that is well-defined.

I used a requires clause in the above example, which requires C++20, for simplicity's sake. However, you can use SFINAE to test for it as well.

Answer 3

I think the goal has indeed been reached. The extra computation time is probably low, since operations on template types are performed at compile time. I'm just wondering about using an enum and bitwise operations rather than using 3 booleans, because I don't think the saving of a few bits is justified, unless you have to store a huge amount of this type of data. I imagine it's in order to plug into legacy code, in which case the approach brings a lot of clarity to the usage.