[Lldb-commits] [PATCH] D74759: Treat RangeDataVector as an augmented BST

[Unnar Freyr Erlendsson via Phabricator via lldb-commits]

unnar added inline comments.


================
Comment at: lldb/include/lldb/Utility/RangeMap.h:642
+  B ComputeUpperBounds(int lo, int hi) {
+    if (lo > hi) return B();
+
----------------
jarin wrote:
> Here, B() should be the min value of type B, no? Perhaps this should be `std::numeric_limits<B>::min()` instead of `B()`?
Removed and made sure not to recursively call in degenerate cases.


================
Comment at: lldb/include/lldb/Utility/RangeMap.h:745
+  void FindEntryIndexesThatContain(B addr, int lo, int hi,
+                                   std::vector<uint32_t> &indexes) {
+    if (lo > hi) return;
----------------
jarin wrote:
> Hmm, weird, I am surprised this is not `std::vector<T> &indexes` (I realize this was in the code before).
I suspect this function used to have a different implementation where it would return the indexes of the entries rather than the data itself similar to FindEntryIndexThatContains and it was not properly changed when it was updated.


================
Comment at: lldb/include/lldb/Utility/RangeMap.h:849
   Compare m_compare;
+  bool upper_bound_computed;
 };
----------------
jarin wrote:
> I am guessing this should have the `m_` prefix?
Removed as it is no longer needed.


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

https://reviews.llvm.org/D74759