[Lldb-commits] [PATCH] D59291: [Object] Add basic minidump support
Zachary Turner via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 14 10:29:04 PDT 2019
zturner added inline comments.
================
Comment at: include/llvm/Object/Minidump.h:77
+ ArrayRef<minidump::Directory> Streams,
+ std::unordered_map<minidump::StreamType, std::size_t> StreamMap)
+ : Binary(ID_Minidump, Source), Header(Header), Streams(Streams),
----------------
jhenderson wrote:
> Are you deliberately making a copy of StreamMap? I would normally expect this to be passed by some form of reference.
It's actually not making a copy. This gives the caller the ability to write `std::move(StreamMap)`, which will actually *prevent* a copy. If you change this to a reference, then in order to store it in the class by value a copy must be made. By passing it by value in the constructor, you can prevent *any* copies from being made, because the caller can move it in at construction time (and indeed, this is what happens later on).
However, I would also ask why we're using `std::unordered_map<>` instead of `llvm::DenseMap<>`, which is generally more efficient.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59291/new/
https://reviews.llvm.org/D59291
More information about the lldb-commits
mailing list