[llvm] Adding bidirectional iterator functionality + unit tests. (PR #160726)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-adt

Author: Francisco Geiman Thiesen (FranciscoThiesen)

<details>
<summary>Changes</summary>

This feature is needed for #<!-- -->160415 , @<!-- -->kuhar suggested that I split that PR into 2 so that the ADT work is checking in first.

---
Full diff: https://github.com/llvm/llvm-project/pull/160726.diff


2 Files Affected:

- (modified) llvm/include/llvm/ADT/BitVector.h (+23-4) 
- (modified) llvm/unittests/ADT/BitVectorTest.cpp (+93) 


``````````diff
diff --git a/llvm/include/llvm/ADT/BitVector.h b/llvm/include/llvm/ADT/BitVector.h
index 72da2343fae13..ef619aef208f3 100644
--- a/llvm/include/llvm/ADT/BitVector.h
+++ b/llvm/include/llvm/ADT/BitVector.h
@@ -40,12 +40,20 @@ template <typename BitVectorT> class const_set_bits_iterator_impl {
     Current = Parent.find_next(Current);
   }
 
+  void retreat() {
+    if (Current == -1) {
+      Current = Parent.find_last();
+    } else {
+      Current = Parent.find_prev(Current);
+    }
+  }
+
 public:
-  using iterator_category = std::forward_iterator_tag;
+  using iterator_category = std::bidirectional_iterator_tag;
   using difference_type   = std::ptrdiff_t;
-  using value_type        = int;
-  using pointer           = value_type*;
-  using reference         = value_type&;
+  using value_type        = unsigned;
+  using pointer           = const value_type*;
+  using reference         = value_type;
 
   const_set_bits_iterator_impl(const BitVectorT &Parent, int Current)
       : Parent(Parent), Current(Current) {}
@@ -64,6 +72,17 @@ template <typename BitVectorT> class const_set_bits_iterator_impl {
     return *this;
   }
 
+  const_set_bits_iterator_impl operator--(int) {
+    auto Prev = *this;
+    retreat();
+    return Prev;
+  }
+
+  const_set_bits_iterator_impl &operator--() {
+    retreat();
+    return *this;
+  }
+
   unsigned operator*() const { return Current; }
 
   bool operator==(const const_set_bits_iterator_impl &Other) const {
diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp
index 6a4780c143e54..216e9a0c579f4 100644
--- a/llvm/unittests/ADT/BitVectorTest.cpp
+++ b/llvm/unittests/ADT/BitVectorTest.cpp
@@ -9,6 +9,7 @@
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallBitVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -1177,6 +1178,98 @@ TYPED_TEST(BitVectorTest, Iterators) {
     EXPECT_EQ(List[i++], Bit);
 }
 
+TYPED_TEST(BitVectorTest, BidirectionalIterator) {
+  // Test decrement operators
+  TypeParam Vec(100, false);
+  Vec.set(10);
+  Vec.set(20);
+  Vec.set(30);
+  Vec.set(40);
+
+  // Test that we can decrement from end()
+  auto EndIt = Vec.set_bits_end();
+  auto LastIt = EndIt;
+  --LastIt;
+  EXPECT_EQ(*LastIt, 40U);
+
+  // Test post-decrement
+  auto It = Vec.set_bits_end();
+  auto PrevIt = It--;
+  EXPECT_EQ(PrevIt, Vec.set_bits_end());
+  EXPECT_EQ(*It, 40U);
+
+  // Test pre-decrement
+  --It;
+  EXPECT_EQ(*It, 30U);
+
+  // Test full backward iteration
+  std::vector<unsigned> BackwardBits;
+  for (auto RIt = Vec.set_bits_end(); RIt != Vec.set_bits_begin(); ) {
+    --RIt;
+    BackwardBits.push_back(*RIt);
+  }
+  EXPECT_EQ(BackwardBits.size(), 4U);
+  EXPECT_EQ(BackwardBits[0], 40U);
+  EXPECT_EQ(BackwardBits[1], 30U);
+  EXPECT_EQ(BackwardBits[2], 20U);
+  EXPECT_EQ(BackwardBits[3], 10U);
+}
+
+TYPED_TEST(BitVectorTest, ReverseIteration) {
+  // Test using llvm::reverse
+  TypeParam Vec(100, false);
+  Vec.set(5);
+  Vec.set(15);
+  Vec.set(25);
+  Vec.set(35);
+  Vec.set(45);
+
+  std::vector<unsigned> ReversedBits;
+  for (unsigned Bit : llvm::reverse(Vec.set_bits())) {
+    ReversedBits.push_back(Bit);
+  }
+
+  EXPECT_EQ(ReversedBits.size(), 5U);
+  EXPECT_EQ(ReversedBits[0], 45U);
+  EXPECT_EQ(ReversedBits[1], 35U);
+  EXPECT_EQ(ReversedBits[2], 25U);
+  EXPECT_EQ(ReversedBits[3], 15U);
+  EXPECT_EQ(ReversedBits[4], 5U);
+}
+
+TYPED_TEST(BitVectorTest, BidirectionalIteratorEdgeCases) {
+  // Test empty BitVector
+  TypeParam Empty;
+  EXPECT_EQ(Empty.set_bits_begin(), Empty.set_bits_end());
+
+  // Decrementing end() on empty should give -1 (no bits set)
+  auto EmptyEndIt = Empty.set_bits_end();
+  --EmptyEndIt;
+  // After decrement on empty, iterator should still be at "no bit" position
+  EXPECT_EQ(*EmptyEndIt, static_cast<unsigned>(-1));
+
+  // Test single bit
+  TypeParam Single(10, false);
+  Single.set(5);
+
+  auto SingleIt = Single.set_bits_end();
+  --SingleIt;
+  EXPECT_EQ(*SingleIt, 5U);
+  // After decrementing past the first element, the iterator is in an
+  // undefined state (before begin), so we don't test this case
+
+  // Test all bits set
+  TypeParam AllSet(10, true);
+  std::vector<unsigned> AllBitsReverse;
+  for (unsigned Bit : llvm::reverse(AllSet.set_bits())) {
+    AllBitsReverse.push_back(Bit);
+  }
+  EXPECT_EQ(AllBitsReverse.size(), 10U);
+  for (unsigned i = 0; i < 10; ++i) {
+    EXPECT_EQ(AllBitsReverse[i], 9 - i);
+  }
+}
+
 TYPED_TEST(BitVectorTest, PushBack) {
   TypeParam Vec(10, false);
   EXPECT_EQ(-1, Vec.find_first());

``````````

</details>


https://github.com/llvm/llvm-project/pull/160726