[PATCH] D74984: [ADT] Add CoalescingBitVector, implemented using IntervalMap [1/3]

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/docs/ProgrammersManual.rst:2347
 
-Unlike the other containers, there are only two bit storage containers, and
-choosing when to use each is relatively straightforward.
+There are four bit storage containers, and choosing when to use each is
+relatively straightforward.
----------------
Even with the next paragraph I find anything but "three" confusing here given that the headline also mentions three.


================
Comment at: llvm/include/llvm/ADT/CoalescingBitVector.h:109
+  /// for the rationale. For a safe set union operation, use \ref operator|=.
+  void set(const ThisT &RHS) { initFromRHS(RHS); }
+
----------------
Why not move the implementation of initFromRHS here and call `set()` everywhere where initFromRHS is called now? It feels weird to refer to an RHS in something that isn't an operator.


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

https://reviews.llvm.org/D74984