lemo marked an inline comment as done.
lemo added a comment.
In https://reviews.llvm.org/D45700#1070491, @amccarth wrote:
> LGTM, but consider highlighting the side effect to `target` when the factory
> makes a Placeholder module.
Good observation: taking a step back, the factory introduces too much coupling,
especially if we want to extend this placeholder module approach to other uses.
Because of this, I reverted back to the standalone
PlaceholderModule::CreateImageSection() approach. Thanks Adrian!
================
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");
----------------
amccarth wrote:
> 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).
Updated the function comment. This is similar to how other places set the
section load address (ex. ObjectFileELF::SetLoadAddress)
https://reviews.llvm.org/D45700
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
