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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
