From 6205ff8c59a7684ff578037f3fcafcf25295b05c Mon Sep 17 00:00:00 2001 From: Francisco Geiman Thiesen Date: Thu, 25 Sep 2025 08:28:09 -0700 Subject: [PATCH 1/5] Adding bidirectional iterator functionality + unit tests. --- llvm/include/llvm/ADT/BitVector.h | 27 ++++++-- llvm/unittests/ADT/BitVectorTest.cpp | 93 ++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) 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 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 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 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 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(-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 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()); From 6926cfb524ad4ebce146cc6b0d2c233297ba2fb7 Mon Sep 17 00:00:00 2001 From: Francisco Geiman Thiesen Date: Thu, 25 Sep 2025 11:00:25 -0700 Subject: [PATCH 2/5] Reverting formatting changes that are not required for my code. --- llvm/unittests/ADT/BitVectorTest.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp index 216e9a0c579f4..a01f55bf7a96c 100644 --- a/llvm/unittests/ADT/BitVectorTest.cpp +++ b/llvm/unittests/ADT/BitVectorTest.cpp @@ -1192,17 +1192,17 @@ TYPED_TEST(BitVectorTest, BidirectionalIterator) { --LastIt; EXPECT_EQ(*LastIt, 40U); - // Test post-decrement + // 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 + // Test pre-decrement. --It; EXPECT_EQ(*It, 30U); - // Test full backward iteration + // Test full backward iteration. std::vector BackwardBits; for (auto RIt = Vec.set_bits_end(); RIt != Vec.set_bits_begin(); ) { --RIt; @@ -1216,7 +1216,7 @@ TYPED_TEST(BitVectorTest, BidirectionalIterator) { } TYPED_TEST(BitVectorTest, ReverseIteration) { - // Test using llvm::reverse + // Test using llvm::reverse. TypeParam Vec(100, false); Vec.set(5); Vec.set(15); @@ -1238,17 +1238,17 @@ TYPED_TEST(BitVectorTest, ReverseIteration) { } TYPED_TEST(BitVectorTest, BidirectionalIteratorEdgeCases) { - // Test empty BitVector + // 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) + // 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 + // After decrement on empty, iterator should still be at "no bit" position. EXPECT_EQ(*EmptyEndIt, static_cast(-1)); - // Test single bit + // Test single bit. TypeParam Single(10, false); Single.set(5); @@ -1256,9 +1256,9 @@ TYPED_TEST(BitVectorTest, BidirectionalIteratorEdgeCases) { --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 + // undefined state (before begin), so we don't test this case. - // Test all bits set + // Test all bits set. TypeParam AllSet(10, true); std::vector AllBitsReverse; for (unsigned Bit : llvm::reverse(AllSet.set_bits())) { From 0640e6980c5b2ea01e078a7319ed0047645d8a3f Mon Sep 17 00:00:00 2001 From: Francisco Geiman Thiesen Date: Thu, 25 Sep 2025 13:05:38 -0700 Subject: [PATCH 3/5] Update llvm/unittests/ADT/BitVectorTest.cpp Co-authored-by: Jakub Kuderski --- llvm/unittests/ADT/BitVectorTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp index a01f55bf7a96c..a2888d02f3aef 100644 --- a/llvm/unittests/ADT/BitVectorTest.cpp +++ b/llvm/unittests/ADT/BitVectorTest.cpp @@ -1179,7 +1179,7 @@ TYPED_TEST(BitVectorTest, Iterators) { } TYPED_TEST(BitVectorTest, BidirectionalIterator) { - // Test decrement operators + // Test decrement operators. TypeParam Vec(100, false); Vec.set(10); Vec.set(20); From 35b9cf8c95c8db074e3ce23561f08badc01b239e Mon Sep 17 00:00:00 2001 From: Francisco Geiman Thiesen Date: Thu, 25 Sep 2025 13:05:46 -0700 Subject: [PATCH 4/5] Update llvm/unittests/ADT/BitVectorTest.cpp Co-authored-by: Jakub Kuderski --- llvm/unittests/ADT/BitVectorTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp index a2888d02f3aef..de7120623f898 100644 --- a/llvm/unittests/ADT/BitVectorTest.cpp +++ b/llvm/unittests/ADT/BitVectorTest.cpp @@ -1186,7 +1186,7 @@ TYPED_TEST(BitVectorTest, BidirectionalIterator) { Vec.set(30); Vec.set(40); - // Test that we can decrement from end() + // Test that we can decrement from end(). auto EndIt = Vec.set_bits_end(); auto LastIt = EndIt; --LastIt; From ceebbfc687ff16ed7c9a50feb405e5f8744127ed Mon Sep 17 00:00:00 2001 From: Francisco Geiman Thiesen Date: Thu, 25 Sep 2025 14:11:49 -0700 Subject: [PATCH 5/5] Fix focused formatting issues (no wide clang-format) --- llvm/include/llvm/ADT/BitVector.h | 8 ++++---- llvm/unittests/ADT/BitVectorTest.cpp | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/ADT/BitVector.h b/llvm/include/llvm/ADT/BitVector.h index ef619aef208f3..83350e6e45846 100644 --- a/llvm/include/llvm/ADT/BitVector.h +++ b/llvm/include/llvm/ADT/BitVector.h @@ -50,10 +50,10 @@ template class const_set_bits_iterator_impl { public: using iterator_category = std::bidirectional_iterator_tag; - using difference_type = std::ptrdiff_t; - using value_type = unsigned; - using pointer = const value_type*; - using reference = value_type; + using difference_type = std::ptrdiff_t; + 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) {} diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp index de7120623f898..12ba0041af551 100644 --- a/llvm/unittests/ADT/BitVectorTest.cpp +++ b/llvm/unittests/ADT/BitVectorTest.cpp @@ -8,8 +8,8 @@ #include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseSet.h" -#include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallBitVector.h" #include "gtest/gtest.h" using namespace llvm; @@ -1204,7 +1204,7 @@ TYPED_TEST(BitVectorTest, BidirectionalIterator) { // Test full backward iteration. std::vector BackwardBits; - for (auto RIt = Vec.set_bits_end(); RIt != Vec.set_bits_begin(); ) { + for (auto RIt = Vec.set_bits_end(); RIt != Vec.set_bits_begin();) { --RIt; BackwardBits.push_back(*RIt); }