[PATCH] D100387: Simplify BitVector code

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/include/llvm/ADT/BitVector.h:82
 
-  MutableArrayRef<BitWord> Bits; // Actual bits.
-  unsigned Size;                 // Size of bitvector in bits.
+  using Storage = std::vector<BitWord>;
+
----------------
Any thoughts on making this `SmallVector<BitWord, 0>` instead?


================
Comment at: llvm/include/llvm/ADT/BitVector.h:168
   bool any() const {
-    for (unsigned i = 0; i < NumBitWords(size()); ++i)
-      if (Bits[i] != 0)
-        return true;
-    return false;
+    return any_of(Bits, [](BitWord Bit) -> bool { return Bit; });
   }
----------------
is preferable imho.


================
Comment at: llvm/include/llvm/ADT/BitVector.h:345
 
-  void reserve(unsigned N) {
-    if (N > getBitCapacity())
-      grow(N);
-  }
+  void reserve(unsigned N) { Bits.reserve(N); }
 
----------------
Missing NumBitWords?


================
Comment at: llvm/include/llvm/ADT/BitVector.h:349
   BitVector &set() {
-    init_words(Bits, true);
+    init_words(Bits.begin(), true);
     clear_unused_bits();
----------------
You only call this API with Bits.begin() and it already accessed Bits.end() internally, which makes for a weirdly asymmetric API. Move the Bits.begin() inside the call?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100387/new/

https://reviews.llvm.org/D100387