[PATCH] D117115: [llvm][ADT] Implement `BitVector::{pop_,}back`

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 12 13:05:01 PST 2022


dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

Given that `BitVector::back()` already exists this makes sense.

LGTM, after you add some `!empty()` assertions for clarity if/when these are misused.



================
Comment at: llvm/include/llvm/ADT/BitVector.h:456
+  /// Return the last element in the vector.
+  bool back() const { return (*this)[size() - 1]; }
+
----------------
I suggest asserting `!empty()` here for clarity of the assertion message.


================
Comment at: llvm/include/llvm/ADT/BitVector.h:480
+  /// Pop one bit from the end of the vector.
+  void pop_back() { resize(size() - 1); }
+
----------------
I suggest asserting `!empty()` here for clarity of the assertion message.


================
Comment at: llvm/include/llvm/ADT/SmallBitVector.h:475
+  /// Return the last element in the vector.
+  bool back() const { return (*this)[size() - 1]; }
+
----------------
I suggest asserting `!empty()` here for clarity of the assertion message.


================
Comment at: llvm/include/llvm/ADT/SmallBitVector.h:487
+  /// Pop one bit from the end of the vector.
+  void pop_back() { resize(size() - 1); }
+
----------------
I suggest asserting `!empty()` here for clarity of the assertion message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117115



More information about the llvm-commits mailing list