[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 10 09:00:42 PST 2018

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




More information about the lldb-commits mailing list