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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this is looking very good now. Just a few polishing comments.



================
Comment at: lldb/include/lldb/Utility/RangeMap.h:604
 
+template <typename B, typename S, typename T>
+struct AugmentedRangeData : public RangeData<B, S, T> {
----------------
Add a short comment about the purpose of this class. Maybe you could just move the comment from the function `ComputeUpperBounds` to here?


================
Comment at: lldb/include/lldb/Utility/RangeMap.h:608-609
+
+  AugmentedRangeData(B base, S size, T data)
+      : RangeData<B, S, T>(base, size, data), upper_bound() {}
+};
----------------
looking at the usage, it would be simpler if this constructor just took a `const RangeData<B, S, T>&` argument, and then initialized the base class using its copy constructor.


================
Comment at: lldb/include/lldb/Utility/RangeMap.h:626-627
+  void Append(const Entry &entry) {
+    AugmentedEntry augmented_entry(entry.base, entry.size, entry.data);
+    m_entries.push_back(augmented_entry);
+  }
----------------
and then here you'd do `m_entries.emplace_back(entry)`


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

https://reviews.llvm.org/D74759