10
\$\begingroup\$

I am creating a C++ header-only library for FlagSet. A FlagSet is a container for booleans with meaning. Usually, it is done with constants (macro, enums, whatever), for example :

//from SDL2
#define SDL_INIT_AUDIO          0x00000010u
#define SDL_INIT_VIDEO          0x00000020u
#define SDL_INIT_EVENTS         0X00000040u
...
Uint32 flags = SDL_INIT_VIDEO | SDL_INIT_AUDIO;
if ((flags & SDL_INIT_VIDEO)) {
    /* video implies events */
    flags |= SDL_INIT_EVENTS;
}

It is not as safe as a scoped enum (enum class), you can mix up flags from one meaning to another (for example, it is possible to do SDL_INIT_AUDIO | SDL_WINDOW_RESIZABLE even if they do not share any logic).

The aim of my FlagSet library is to provide a "scoped flags" (by analogy with scoped enum). Here is my code :

#include <iostream>
#include <cstddef>
#include <type_traits>
#include <string>

/////////// INDEX OF ///////////

template<typename S,typename...>
struct IndexOf_t {};

template<typename S, typename...Ts>
struct IndexOf_t<S,S,Ts...> {
    static constexpr std::size_t index = 0;
};

template<typename S, typename T, typename...Ts>
struct IndexOf_t<S,T,Ts...> {
    static constexpr std::size_t index = 1 + IndexOf_t<S,Ts...>::index;
};

template<typename Searching, typename...List>
constexpr std::size_t IndexOf = IndexOf_t<Searching,List...>::index;

////////////////////////////////



/////////// FLAGSET ////////////

template <typename U, typename...Fs>
struct FlagSet {
    using underlying_t = std::enable_if_t<std::is_integral_v<U>,U>;

    constexpr FlagSet() : mData{0} {};

    template<typename...Ts>
    static constexpr FlagSet<U,Fs...> Value() {
        return FlagSet { Mask<Ts...> };
    };

    template<typename...Ts>
    constexpr void set() {
        mData |= Mask<Ts...>;
    };

    template<typename...Ts>
    constexpr void reset() {
        mData &= ~Mask<Ts...>;
    };

    template<typename...Ts>
    constexpr void toggle() {
        mData ^= Mask<Ts...>;
    };

    constexpr void setAll() {
        set<Fs...>();
    };

    constexpr void resetAll() {
        reset<Fs...>();
    };

    constexpr void toggleAll() {
        toggle<Fs...>();
    };


    template<typename...Ts>
    constexpr bool anyOf() const {
        return (mData & Mask<Ts...>) != 0;
    };
    template<typename...Ts>
    constexpr bool allOf() const {
        return (mData & Mask<Ts...>) == Mask<Ts...>;
    };
    template<typename...Ts>
    constexpr bool noneOf() const {
        return (mData & Mask<Ts...>) == 0;
    };

    constexpr bool all() const { return allOf<Fs...>(); };
    constexpr bool any() const { return anyOf<Fs...>(); };
    constexpr bool none() const { return noneOf<Fs...>(); };


    std::string to_string(char zero = '0', char one = '1') const {
        std::string str(sizeof...(Fs),zero);
        std::size_t count = 0;
        for (U data = mData; data != 0 && count < sizeof...(Fs); data >>= 1, ++count)
            if (data & 1) str[count] = one;
        return str;
    }

    template<typename U_, typename...Fs_>
    friend std::ostream& operator<<(std::ostream& os, FlagSet<U_,Fs_...> fs);

private:

    U mData;

    constexpr FlagSet(U data) : mData{data} {};

    template<typename...>
    struct Mask_t { 
        static constexpr U mask = 0;
    };
    template<typename T>
    struct Mask_t<T> {
        static constexpr U mask = 1 << IndexOf<T,Fs...>;
    };
    template<typename T, typename...Ts>
    struct Mask_t<T,Ts...> {
        static constexpr U mask = Mask_t<T>::mask | Mask_t<Ts...>::mask;
    };

    template<typename...Ts>
    static constexpr U Mask = Mask_t<Ts...>::mask;


};

template<typename U, typename...Fs>
std::ostream& operator<<(std::ostream& os, FlagSet<U,Fs...> fs) {
    return os << fs.to_string();
};


////////////////////////////////

namespace Direction {
    struct UP;
    struct DOWN;
    struct LEFT;
    struct RIGHT;
    using Type = FlagSet<char, UP, DOWN, LEFT, RIGHT>;
};

int main() {
    using namespace std;

    Direction::Type player_dir = Direction::Type::Value<Direction::DOWN>();

    cout << "display : UP DOWN LEFT RIGHT " << endl;
    cout << "player1 : " << player_dir << endl; // 0100
    player_dir.set<Direction::LEFT>(); //now the player goes left too
    cout << "player2 : " << player_dir << endl; // 0110
    player_dir.resetAll(); //now the player goes nowhere
    cout << "player3 : " << player_dir << endl; // 0000
    player_dir.set<Direction::RIGHT>(); //now the player goes right
    cout << "player4 : " << player_dir << endl; // 0001
    player_dir.toggle<Direction::RIGHT,Direction::UP>(); //now the player goes only up
    cout << "player5 : " << player_dir << endl; // 1000

    if (player_dir.any()) //player is moving
        cout << "player is moving !\n"; //TRUE
    if (player_dir.noneOf<Direction::LEFT,Direction::RIGHT>()) //player is not moving on x-axis
        cout << "player not moving on x-axis\n"; //TRUE
    if (player_dir.noneOf<Direction::UP,Direction::DOWN>()) //player is not moving on y-axis
        cout << "player not moving on y-axis\n"; //FALSE
    return 0;
};

Thank you for anyone reviewing my code !

\$\endgroup\$
4
\$\begingroup\$

It's a great idea to provide a type-safe bit-mask, I like your idea a lot. That said, I feel like your code is in great need of modernization and simplification.

Recursion through inheritance is now a testimony to the ingenuity of our great predecessors, but it's time to get acquainted with if constexpr and fold expressions. I agree that you'll need C++17, but it's 2018 after all. For instance, here's your index_of as a two-liner:

#include <type_traits>
template <typename T, typename U, typename... Ts>
constexpr std::size_t index_of() {
    if constexpr (std::is_same_v<T, U>) return 0;
    else return 1 + index_of<T, Ts...>(); // doesn't have to be well-formed if U == V
}

N.B: As a function qualifier, constexpr means that, given compile-time arguments, the function will be executed at compile-time.

In the same spirit, you can simplify Mask:

template <typename U, typename... Ts>
struct Flag_set {
    // ...
    template <typename... Us>
    constexpr std::size_t mask() {
        return ((1 << index_of<Us, Ts...>()) | ...); // fold expression
    }
    // ...
    U flags;
};

((1 << index_of<Us, Ts...>()) | ...) will be expanded as:

  (1 << index_of<Us1, Ts...>())
| (1 << index_of<Us2, Ts...>())  
| ...                           
| (1 << index_of<UsN, Ts...>()))

Yet another function you can simplify, to_string():

template <typename U, typename... Ts>
struct Flag_set {
    // ... 
    std::string to_string() {
        std::string result;
        (result.push_back(mask<Ts>() & flags ? '1' : '0'), ...); // fold
        std::reverse(result.begin(), result.end());
        return result;
    }
    // ...
    U flags;
};

Here, the fold expression is expanded around the comma operator:

result.push_back(mask<Ts1>() & flags ? '1' : '0'),
result.push_back(mask<Ts2>() & flags ? '1' : '0'),
...
result.push_back(mask<TsN>() & flags ? '1' : '0');

Here's a minimal working example you can toy with and enhance with static assertions and such:

#include <string>
#include <iostream>
#include <type_traits>
#include <algorithm>

template <typename T, typename U, typename... Ts>
constexpr std::size_t index_of() {
    if constexpr (std::is_same_v<T, U>) return 0;
    else return 1 + index_of<T, Ts...>();
}

template <typename U, typename... Ts>
struct Flag_set {

    Flag_set() : flags(0) {}

    template <typename... Us>
    constexpr std::size_t mask() {
        return ((1 << index_of<Us, Ts...>()) | ...);
    }

    template <typename... Us>
    constexpr void set() {
        flags |= mask<Us...>();
    }

    std::string to_string() {
        std::string result;
        (result.push_back(mask<Ts>() & flags ? '1' : '0'), ...);
        std::reverse(result.begin(), result.end());
        return result;
    }

    U flags;
};

int main() {
    Flag_set<std::size_t, char, int, float, double> fs;
    std::cout << fs.mask<char, float>() << '\n';
    fs.set<int, double>();
    std::cout << fs.to_string() << '\n';
}
\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer ! I don't fear C++17, indeed I want to master it, so your answer is really helping me ! I heard about fold expressions but I don't really get it, so thanks for providing me a minimal working example ! if constexpr is really something I will use. \$\endgroup\$ – Julien Vernay Jun 21 '18 at 7:07
10
\$\begingroup\$

IndexOf can probably be a nested type of FlagSet, unless you intend for that to be part of the public interface.

using underlying_t = std::enable_if_t<std::is_integral_v<U>,U>;

Are you sure SFINAE is what you want to do here? It would probably make more sense to static_assert that U is integral.

At the same time, you might also want to static assert that U is unsigned, because you're doing bit twiddling, which is a dodgy prospect with signed types.

And also at the same time, you could static assert that sizeof...(Fs) is less than or equal to the number of bits in U.

Even better, perhaps, would be to static assert that U is an unsigned, possibly fundamental, integral type or a std::bitset or something else with the bit-twiddling interface you need.

Most every function could also probably benefit from being noexcept.

template<typename U_, typename...Fs_>
friend std::ostream& operator<<(std::ostream& os, FlagSet<U_,Fs_...> fs);

This doesn't really need to be a friend, because it's only using the public interface.

What I would suggest, though, is a function that writes the string form of the flag set into a buffer. Something like:

template <typename CharT>
constexpr auto write(CharT* buffer, std::size_t size, CharT zero = CharT('0'), CharT one = CharT('1')) noexcept
{
    // write zero and one into buffer[0] through
    // buffer[min(size, sizeof...(Fs)], basically as you do in to_string
}

With that function, you could implement both to_string() and the insertion operator:

auto to_string(char zero = '0', char one = '1') const
{
    auto s = std::string(sizeof...(fs), zero);
    write(s.data(), s.size(), zero, one);
    return s;
}

template <typename CharT, typename Traits, typename U, typename... Fs>
auto operator<<(std::basic_ostream<CharT, Traits>& o, FlagSet<U, Fs...> fs) ->
    std::basic_ostream<CharT, Traits>&
{
    auto buf = std::array<CharT, sizeof...(Fs)>{};
    write(buf.data(), buf.size());
    o.write(buf.data(), buf.size());
    return o;
}

And you'll even get support for streams of other character types, too.

The only other thing I can think off of the top of my head is that you'll probably want operator== and !=. And maybe a way to set the flag set from either a bag o' bits (like an unsigned type or std::bitset) or by reading a string (as in, the complement of operator<<). Because otherwise, setting the flags using data read from a config file (for example) would be a pain.

\$\endgroup\$
  • \$\begingroup\$ Would be great to stress on importance of using static_assert here, as it is detrimental for useful error messages. +1 anyway, though. \$\endgroup\$ – Incomputable Jun 20 '18 at 4:21
  • \$\begingroup\$ Thanks for your feedback, static_assert is something I will use ! For the write function, why not even if I don't like the (raw pointer, size) pattern (I consider using GSL::span ?). Using operator<< would be great, as you say for serialization it would be useful. \$\endgroup\$ – Julien Vernay Jun 21 '18 at 7:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.