Add Flags<F>::Iterator.

This simplifies the string code and seems generally useful for people
that want to operate on individual flags.

Bug: 160010896
Test: atest libinput_tests
Change-Id: I13aa913eb85d6294b2bf3c899a6a1ab700a40374

2 files changed, 126 insertions, 26 deletions

diff --git a/include/input/Flags.h b/include/input/Flags.h
index f3198c9e91..0f52e18719 100644
--- a/include/input/Flags.h
+++ b/include/input/Flags.h
@@ -50,61 +50,123 @@ class Flags {
     using U = typename std::underlying_type_t<F>;
 
 public:
-    constexpr Flags(F f) : flags(static_cast<U>(f)) {}
-    constexpr Flags() : flags(0) {}
-    constexpr Flags(const Flags<F>& f) : flags(f.flags) {}
+    constexpr Flags(F f) : mFlags(static_cast<U>(f)) {}
+    constexpr Flags() : mFlags(0) {}
+    constexpr Flags(const Flags<F>& f) : mFlags(f.mFlags) {}
 
     // Provide a non-explicit construct for non-enum classes since they easily convert to their
     // underlying types (e.g. when used with bitwise operators). For enum classes, however, we
     // should force them to be explicitly constructed from their underlying types to make full use
     // of the type checker.
     template <typename T = U>
-    constexpr Flags(T t, typename std::enable_if_t<!is_enum_class_v<F>, T>* = nullptr) : flags(t) {}
+    constexpr Flags(T t, typename std::enable_if_t<!is_enum_class_v<F>, T>* = nullptr)
+          : mFlags(t) {}
     template <typename T = U>
     explicit constexpr Flags(T t, typename std::enable_if_t<is_enum_class_v<F>, T>* = nullptr)
-          : flags(t) {}
+          : mFlags(t) {}
+
+    class Iterator {
+        // The type can't be larger than 64-bits otherwise it won't fit in BitSet64.
+        static_assert(sizeof(U) <= sizeof(uint64_t));
+
+    public:
+        Iterator(Flags<F> flags) : mRemainingFlags(flags.mFlags) { (*this)++; }
+        Iterator() : mRemainingFlags(0), mCurrFlag(static_cast<F>(0)) {}
+
+        // Pre-fix ++
+        Iterator& operator++() {
+            if (mRemainingFlags.isEmpty()) {
+                mCurrFlag = static_cast<F>(0);
+            } else {
+                uint64_t bit = mRemainingFlags.clearLastMarkedBit(); // counts from left
+                const U flag = 1 << (64 - bit - 1);
+                mCurrFlag = static_cast<F>(flag);
+            }
+            return *this;
+        }
+
+        // Post-fix ++
+        Iterator operator++(int) {
+            Iterator iter = *this;
+            ++*this;
+            return iter;
+        }
+
+        bool operator==(Iterator other) const {
+            return mCurrFlag == other.mCurrFlag && mRemainingFlags == other.mRemainingFlags;
+        }
+
+        bool operator!=(Iterator other) const { return !(*this == other); }
+
+        F operator*() { return mCurrFlag; }
+
+        // iterator traits
+
+        // In the future we could make this a bidirectional const iterator instead of a forward
+        // iterator but it doesn't seem worth the added complexity at this point. This could not,
+        // however, be made a non-const iterator as assigning one flag to another is a non-sensical
+        // operation.
+        using iterator_category = std::input_iterator_tag;
+        using value_type = F;
+        // Per the C++ spec, because input iterators are not assignable the iterator's reference
+        // type does not actually need to be a reference. In fact, making it a reference would imply
+        // that modifying it would change the underlying Flags object, which is obviously wrong for
+        // the same reason this can't be a non-const iterator.
+        using reference = F;
+        using difference_type = void;
+        using pointer = void;
+
+    private:
+        BitSet64 mRemainingFlags;
+        F mCurrFlag;
+    };
+
     /*
      * Tests whether the given flag is set.
      */
     bool test(F flag) const {
         U f = static_cast<U>(flag);
-        return (f & flags) == f;
+        return (f & mFlags) == f;
     }
 
     /* Tests whether any of the given flags are set */
-    bool any(Flags<F> f) { return (flags & f.flags) != 0; }
+    bool any(Flags<F> f) { return (mFlags & f.mFlags) != 0; }
 
     /* Tests whether all of the given flags are set */
-    bool all(Flags<F> f) { return (flags & f.flags) == f.flags; }
+    bool all(Flags<F> f) { return (mFlags & f.mFlags) == f.mFlags; }
 
-    Flags<F> operator|(Flags<F> rhs) const { return static_cast<F>(flags | rhs.flags); }
+    Flags<F> operator|(Flags<F> rhs) const { return static_cast<F>(mFlags | rhs.mFlags); }
     Flags<F>& operator|=(Flags<F> rhs) {
-        flags = flags | rhs.flags;
+        mFlags = mFlags | rhs.mFlags;
         return *this;
     }
 
-    Flags<F> operator&(Flags<F> rhs) const { return static_cast<F>(flags & rhs.flags); }
+    Flags<F> operator&(Flags<F> rhs) const { return static_cast<F>(mFlags & rhs.mFlags); }
     Flags<F>& operator&=(Flags<F> rhs) {
-        flags = flags & rhs.flags;
+        mFlags = mFlags & rhs.mFlags;
         return *this;
     }
 
-    Flags<F> operator^(Flags<F> rhs) const { return static_cast<F>(flags ^ rhs.flags); }
+    Flags<F> operator^(Flags<F> rhs) const { return static_cast<F>(mFlags ^ rhs.mFlags); }
     Flags<F>& operator^=(Flags<F> rhs) {
-        flags = flags ^ rhs.flags;
+        mFlags = mFlags ^ rhs.mFlags;
         return *this;
     }
 
-    Flags<F> operator~() { return static_cast<F>(~flags); }
+    Flags<F> operator~() { return static_cast<F>(~mFlags); }
 
-    bool operator==(Flags<F> rhs) const { return flags == rhs.flags; }
+    bool operator==(Flags<F> rhs) const { return mFlags == rhs.mFlags; }
     bool operator!=(Flags<F> rhs) const { return !operator==(rhs); }
 
     Flags<F>& operator=(const Flags<F>& rhs) {
-        flags = rhs.flags;
+        mFlags = rhs.mFlags;
         return *this;
     }
 
+    Iterator begin() const { return Iterator(*this); }
+
+    Iterator end() const { return Iterator(); }
+
     /*
      * Returns the stored set of flags.
      *
@@ -112,24 +174,20 @@ public:
      * the value is no longer necessarily a strict member of the enum since the returned value could
      * be multiple enum variants OR'd together.
      */
-    U get() const { return flags; }
+    U get() const { return mFlags; }
 
     std::string string() const { return string(defaultStringify); }
 
     std::string string(std::function<std::optional<std::string>(F)> stringify) const {
-        // The type can't be larger than 64-bits otherwise it won't fit in BitSet64.
-        static_assert(sizeof(U) <= sizeof(uint64_t));
         std::string result;
         bool first = true;
         U unstringified = 0;
-        for (BitSet64 bits(flags); !bits.isEmpty();) {
-            uint64_t bit = bits.clearLastMarkedBit(); // counts from left
-            const U flag = 1 << (64 - bit - 1);
-            std::optional<std::string> flagString = stringify(static_cast<F>(flag));
+        for (const F f : *this) {
+            std::optional<std::string> flagString = stringify(f);
             if (flagString) {
                 appendFlag(result, flagString.value(), first);
             } else {
-                unstringified |= flag;
+                unstringified |= static_cast<U>(f);
             }
         }
 
@@ -145,7 +203,7 @@ public:
     }
 
 private:
-    U flags;
+    U mFlags;
 
     static std::optional<std::string> defaultStringify(F) { return std::nullopt; }
     static void appendFlag(std::string& str, const std::string& flag, bool& first) {
diff --git a/libs/input/tests/Flags_test.cpp b/libs/input/tests/Flags_test.cpp
index 800404df59..b979f37b86 100644
--- a/libs/input/tests/Flags_test.cpp
+++ b/libs/input/tests/Flags_test.cpp
@@ -197,4 +197,46 @@ TEST(Flags, String_WithIncompleteStringify) {
     ASSERT_EQ(flags.string(toStringIncomplete), "ONE | 0x00000004");
 }
 
+TEST(FlagsIterator, IteratesOverAllFlags) {
+    Flags<TestFlags> flags1 = TestFlags::ONE | TestFlags::TWO;
+    Flags<TestFlags> flags2;
+    for (TestFlags f : flags1) {
+        flags2 |= f;
+    }
+    ASSERT_EQ(flags2, flags1);
+}
+
+TEST(FlagsIterator, IteratesInExpectedOrder) {
+    const std::vector<TestFlags> flagOrder = {TestFlags::ONE, TestFlags::TWO};
+    Flags<TestFlags> flags;
+    for (TestFlags f : flagOrder) {
+        flags |= f;
+    }
+
+    size_t idx = 0;
+    auto iter = flags.begin();
+    while (iter != flags.end() && idx < flagOrder.size()) {
+        // Make sure the order is what we expect
+        ASSERT_EQ(*iter, flagOrder[idx]);
+        iter++;
+        idx++;
+    }
+    ASSERT_EQ(iter, flags.end());
+}
+TEST(FlagsIterator, PostFixIncrement) {
+    Flags<TestFlags> flags = TestFlags::ONE | TestFlags::TWO;
+    auto iter = flags.begin();
+    ASSERT_EQ(*(iter++), TestFlags::ONE);
+    ASSERT_EQ(*iter, TestFlags::TWO);
+    ASSERT_EQ(*(iter++), TestFlags::TWO);
+    ASSERT_EQ(iter, flags.end());
+}
+
+TEST(FlagsIterator, PreFixIncrement) {
+    Flags<TestFlags> flags = TestFlags::ONE | TestFlags::TWO;
+    auto iter = flags.begin();
+    ASSERT_EQ(*++iter, TestFlags::TWO);
+    ASSERT_EQ(++iter, flags.end());
+}
+
 } // namespace android::test
\ No newline at end of file