Common: Minor cleanup of flag helper class

Signed-off-by: Felix Schlepper <felix.schlepper@cern.ch>

Author: f3sch

Common/Utils/include/CommonUtils/EnumFlags.h

@@ -144,8 +144,8 @@ struct FlagsHelper final {
   {
     constexpr std::array<bool, sizeof...(I)> valid{isValid<static_cast<E>(MinScan + I)>()...};
     constexpr auto count{std::count_if(valid.cbegin(), valid.cend(), [](bool v) noexcept { return v; })};
-    static_assert(count > 0, "Requiring non-empty enum!");
-    static_assert(count <= MaxUnderScan, "Underlying type of enum has less digits than given expected!");
+    static_assert(count > 0, "EnumFlag requires at least one enum value. Check that your enum has consecutive values starting from 0.");
+    static_assert(count <= MaxUnderScan, "Too many enum values for underlying type. Consider using a larger underlying type or fewer enum values.");
     std::array<E, count> values{};
     for (size_t idx{}, n{}; n < count; ++idx) {
       if (valid[idx]) {
@@ -173,7 +173,7 @@ struct FlagsHelper final {
     }
     if constexpr (tpeek_v<e>[tp + getSpec<SVal::Start, SType::Enum_t>().size()] == getSpec<SVal::AnonStart, SType::Enum_t>()) {
 #if defined __clang__
-      if constexpr (tpeek_v<e>[tp + getSpec<SVal::Start, SType::enum_t>().size() + 1] == getSpec<SVal::AnonStart, SType::Enum_t>()) {
+      if constexpr (tpeek_v<e>[tp + getSpec<SVal::Start, SType::Enum_t>().size() + 1] == getSpec<SVal::AnonStart, SType::Enum_t>()) {
         return {};
       }
 #endif
@@ -215,7 +215,7 @@ struct FlagsHelper final {
   template <E e>
   static constexpr auto getNameValue{getName<e>()};
 
-  template <bool with_scope, std::size_t... I>
+  template <bool with_scope, size_t... I>
   static constexpr auto getNames(std::index_sequence<I...> /*unused*/)
   {
     if constexpr (with_scope) {
@@ -248,7 +248,7 @@ struct FlagsHelper final {
 
   static constexpr std::optional<E> fromString(std::string_view str) noexcept
   {
-    for (std::size_t i{0}; i < count(); ++i) {
+    for (size_t i{0}; i < count(); ++i) {
       if (Names[i] == str || NamesScoped[i] == str) {
         return Values[i];
       }
@@ -371,13 +371,18 @@ class EnumFlags
   constexpr EnumFlags(const EnumFlags&) = default;
   // Move constructor.
   constexpr EnumFlags(EnumFlags&&) = default;
-  // Constructor to initialize with the underlyiny type.
+  // Constructor to initialize with the underlying type.
   constexpr explicit EnumFlags(U u) : mBits(u) {}
   // Initialize with a list of flags.
   constexpr EnumFlags(std::initializer_list<E> flags) noexcept
   {
     std::for_each(flags.begin(), flags.end(), [this](const E f) noexcept { mBits |= to_bit(f); });
   }
+  // Init from a string.
+  EnumFlags(const std::string& str)
+  {
+    set(str);
+  }
   // Destructor.
   constexpr ~EnumFlags() = default;
 
@@ -415,7 +420,7 @@ class EnumFlags
     }
   }
   // Returns the raw bitset value.
-  constexpr auto value() const noexcept
+  [[nodiscard]] constexpr auto value() const noexcept
   {
     return mBits;
   }
@@ -442,6 +447,13 @@ class EnumFlags
     return (mBits & to_bit(t)) != None;
   }
 
+  // Tests if all specified flags are set.
+  template <typename... Ts>
+  [[nodiscard]] constexpr bool test(Ts... flags) const noexcept
+  {
+    return ((test(flags) && ...));
+  }
+
   // Sets a specific flag.
   template <typename T>
     requires std::is_same_v<T, E>
@@ -505,27 +517,27 @@ class EnumFlags
   }
 
   // Checks if any flag is set (Boolean context).
-  constexpr explicit operator bool() const noexcept
+  [[nodiscard]] constexpr explicit operator bool() const noexcept
   {
     return any();
   }
 
   // Check if given flag is set.
   template <typename T>
     requires std::is_same_v<T, E>
-  constexpr bool operator[](const T t) noexcept
+  [[nodiscard]] constexpr bool operator[](const T t) const noexcept
   {
     return test(t);
   }
 
   // Checks if two flag sets are equal.
-  constexpr bool operator==(const EnumFlags& o) const noexcept
+  [[nodiscard]] constexpr bool operator==(const EnumFlags& o) const noexcept
   {
     return mBits == o.mBits;
   }
 
   // Checks if two flag sets are not equal.
-  constexpr bool operator!=(const EnumFlags& o) const noexcept
+  [[nodiscard]] constexpr bool operator!=(const EnumFlags& o) const noexcept
   {
     return mBits != o.mBits;
   }
@@ -584,7 +596,13 @@ class EnumFlags
   // Performs a bitwise XOR with another flag set.
   constexpr EnumFlags operator^(const EnumFlags& o) const noexcept
   {
-    return Flags(mBits ^ o.mBits);
+    return EnumFlags(mBits ^ o.mBits);
+  }
+
+  // Performs a bitwise and with another flag set.
+  constexpr EnumFlags operator&(const EnumFlags& o) const noexcept
+  {
+    return EnumFlags(mBits & o.mBits);
   }
 
   // Performs a bitwise XOR assignment.
@@ -596,14 +614,14 @@ class EnumFlags
 
   // Checks if all specified flags are set.
   template <typename... Ts>
-  constexpr bool all_of(Ts... flags) const noexcept
+  [[nodiscard]] constexpr bool all_of(Ts... flags) const noexcept
   {
-    return ((test(flags) && ...));
+    return test(flags...);
   }
 
   // Checks if none of the specified flags are set.
   template <typename... Ts>
-  constexpr bool none_of(Ts... flags) const noexcept
+  [[nodiscard]] constexpr bool none_of(Ts... flags) const noexcept
   {
     return (!(test(flags) || ...));
   }
@@ -627,35 +645,29 @@ class EnumFlags
   // Counts the number of set bits (active flags).
   [[nodiscard]] constexpr size_t count() const noexcept
   {
-    size_t c{0};
-    for (size_t i{H::Min_u_v}; i < H::Max_u_v; ++i) {
-      if ((mBits & (U(1) << i)) != U(0)) {
-        ++c;
-      }
-    }
-    return c;
+    return std::popcount(mBits);
   }
 
   // Returns the union of two flag sets.
-  constexpr EnumFlags union_with(const EnumFlags& o) const noexcept
+  [[nodiscard]] constexpr EnumFlags union_with(const EnumFlags& o) const noexcept
   {
     return EnumFlags(mBits | o.mBits);
   }
 
   // Returns the intersection of two flag sets.
-  constexpr EnumFlags intersection_with(const EnumFlags& o) const noexcept
+  [[nodiscard]] constexpr EnumFlags intersection_with(const EnumFlags& o) const noexcept
   {
     return EnumFlags(mBits & o.mBits);
   }
 
   // Checks if all flags in another Flags object are present in the current object.
-  constexpr bool contains(const EnumFlags& other) const noexcept
+  [[nodiscard]] constexpr bool contains(const EnumFlags& other) const noexcept
   {
     return (mBits & other.mBits) == other.mBits;
   }
 
  private:
-  // Set implemnetation, bits was zeroed before.
+  // Set implementation, bits was zeroed before.
   void setImpl(const std::string& s, int base = 2)
   {
     if (std::all_of(s.begin(), s.end(), [](unsigned char c) { return std::isdigit(c); })) {
@@ -669,15 +681,20 @@ class EnumFlags
         throw std::out_of_range("Values exceeds enum range.");
       }
       mBits = static_cast<U>(v);
-    } else if (std::all_of(s.begin(), s.end(), [](unsigned char c) { return std::isalnum(c) != 0 || c == '|' || c == ' ' || c == ':' || c == ','; })) {
+    } else if (std::all_of(s.begin(), s.end(), [](unsigned char c) { return std::isalnum(c) != 0 || c == '|' || c == ' ' || c == ':' || c == ',' || c == ';'; })) {
       std::string cs{s};
       std::transform(cs.begin(), cs.end(), cs.begin(), [](unsigned char c) { return std::tolower(c); });
       if (cs == H::All) {
         mBits = All;
       } else if (cs == H::None) {
         mBits = None;
       } else {
-        char token = (s.find(',') != std::string::npos) ? ',' : '|';
+        // accept as delimiter ' ', '|', ';', ','
+        char token = ' ';
+        std::string::size_type pos = s.find_first_of(",|;");
+        if (pos != std::string::npos) {
+          token = s[pos];
+        }
         for (const auto& tok : Str::tokenize(s, token)) {
           if (auto e = H::fromString(tok)) {
             mBits |= to_bit(*e);

Common/Utils/test/testEnumFlags.cxx

@@ -181,7 +181,7 @@ BOOST_AUTO_TEST_CASE(Flags_test)
       BOOST_TEST(flags.test(TestEnum::Bit4));
     }
 
-    { // test with different delimiter
+    { // test with , delimiter
       std::string str = "Bit4,TestEnum::Bit2 , Bit1 ";
       flags.set(str);
       BOOST_TEST(flags.test(TestEnum::Bit1));
@@ -190,6 +190,15 @@ BOOST_AUTO_TEST_CASE(Flags_test)
       BOOST_TEST(flags.test(TestEnum::Bit4));
     }
 
+    { // test with ; delimiter
+      std::string str = "Bit4;TestEnum::Bit2 ; Bit1 ";
+      flags.set(str);
+      BOOST_TEST(flags.test(TestEnum::Bit1));
+      BOOST_TEST(flags.test(TestEnum::Bit2));
+      BOOST_TEST(!flags.test(TestEnum::Bit3));
+      BOOST_TEST(flags.test(TestEnum::Bit4));
+    }
+
     { // throw test with mixed delimiter
       std::string str = "Bit4|TestEnum::Bit2 , Bit1 ";
       BOOST_CHECK_THROW(flags.set(str), std::invalid_argument);
@@ -275,6 +284,14 @@ BOOST_AUTO_TEST_CASE(Flags_test)
     EFlags flags3{TestEnum::Bit1, TestEnum::Bit2, TestEnum::Bit3};
     EFlags flags4{TestEnum::Bit2, TestEnum::Bit3, TestEnum::Bit4};
 
+    // test xor
+    auto flagsXOR = flags3 ^ flags4;
+    BOOST_CHECK(flagsXOR.test(TestEnum::Bit1, TestEnum::Bit4));
+
+    // test and
+    auto flagsAND = flags3 & flags4;
+    BOOST_CHECK(flagsAND.test(TestEnum::Bit2, TestEnum::Bit3));
+
     // Perform an intersection operation
     EFlags intersectionFlags = flags3.intersection_with(flags4);
     BOOST_CHECK(intersectionFlags.test(TestEnum::Bit2));
@@ -300,8 +317,9 @@ BOOST_AUTO_TEST_CASE(Flags_test)
 
   {
     // Test compilation using an enum with more than 32 bits
-    o2::utils::EnumFlags<TestEnumLong> test;
-    test.set("Bit32");
-    BOOST_CHECK(test.test(TestEnumLong::Bit32));
+    // Also tests space delimiter and construction from string.
+    o2::utils::EnumFlags<TestEnumLong> test("Bit32 Bit34");
+    BOOST_CHECK(test.test(TestEnumLong::Bit32, TestEnumLong::Bit34));
+    BOOST_CHECK(!test.test(TestEnumLong::Bit1, TestEnumLong::Bit23));
   }
 }