labath added a comment.

Woohoo for speedups.

However, I noticed that you are modifying the signatures of SB functions, which 
I don't believe will be acceptable for lldb.



================
Comment at: include/lldb/API/SBMemoryRegionInfo.h:105
 
-  SBMemoryRegionInfo(const lldb_private::MemoryRegionInfo *lldb_object_ptr);
+  SBMemoryRegionInfo(lldb::MemoryRegionInfoUP &&lldb_object_up);
+
----------------
Generally, I think it's better to use values here instead of rvalue references, 
as they make clearer interfaces (a function which receives the a unique_ptr by 
value has to consume it whether it likes to or not, but in case of rvalue 
references, the object will survive the function call unless the function 
explicitly does something to it).

I don't believe this should matter for performance, as that's just copying the 
pointer value around.


================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:404
 
+namespace {
+  constexpr uint32_t StateMemFree =
----------------
Namespaces are not indented in llvm style.


================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:408
+
+  MemoryRegionInfo MemoryInfoToRegionInfo(const MinidumpMemoryInfo &entry) {
+  const auto yes = MemoryRegionInfo::eYes;
----------------
llvm prefers static functions instead of ones in anonymous namespaces


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55472



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to