[PATCH] D33104: [BitVector] Implement find_[first/last]_[set/unset]_in

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

Very nice generally. Comments only on one, but clearly apply to each variation.

Bit of a shame there isn't a clean way to share all the logic, but all the ideas to do that are *awful*. =/



================
Comment at: llvm/include/llvm/ADT/BitVector.h:151
+  /// [Begin, End).  Returns -1 if all bits in the range are unset.
+  int find_first_in(unsigned Begin, unsigned End) const {
+    if (Begin >= End)
----------------
Given that we return an int, I would prefer that the arguments also be int (and the intermediate variables within).


================
Comment at: llvm/include/llvm/ADT/BitVector.h:152-153
+  int find_first_in(unsigned Begin, unsigned End) const {
+    if (Begin >= End)
       return -1;
 
----------------
Why isn't this an assert?


https://reviews.llvm.org/D33104