[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.
This revision is now accepted and ready to land.

LGTM, but consider highlighting the side effect to `target` when the factory makes a Placeholder module.

In the future, we need to refactor the minidump tests to get rid of the redundancy.



================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:70
+  // Creates a synthetic module section covering the whole module image
+  void CreateImageSection(const MinidumpModule *module, Target& target) {
+    const ConstString section_name(".module_image");
----------------
I didn't notice before that target is a non-const ref.  Maybe the comment should explain why target is modified (and/or maybe in PlaceholderModule::Create).


https://reviews.llvm.org/D45700