[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added inline comments.


================
Comment at: include/lldb/Core/RangeMap.h:636
 
-template <typename B, typename S, typename T, unsigned N> class RangeDataArray {
+template <typename B, typename S, typename T, unsigned N>
+class RangeDataVector {
----------------
tberghammer wrote:
> Would it make sense to have a default value of 0 for N so people don't have to specify it explicitly?
I don't have an clear opinion on that. On one hand, 0 seems like a perfectly reasonable default, but on the other SmallVector doesn't have a default either. Everyone typedefs these anyway, so it doesn't really matter.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.h:140
+  typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, FileRange,
+                                        0>
       VMRangeToFileOffset;
----------------
tberghammer wrote:
> Should this be 1 to keep the previous semantics?
If we wanted to be strict then yes, but it seemed weird to have different sizes for the two VMRange maps, when both of them will (hopefully) hold the same number of entries. My guess is that the author here meant "the smallest number possible" and didn't realize that 0 is valid too (or maybe it wasn't valid at that time).


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

https://reviews.llvm.org/D56170