[Lldb-commits] [PATCH] D23545: Minidump parsing

[Zachary Turner via lldb-commits]

zturner added inline comments.

================
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:67
@@ +66,3 @@
+    bool m_valid_data;
+    llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> m_directory_map;
+};
----------------
dvlahovski wrote:
> zturner wrote:
> > Can this be `llvm::DenseMap<MinidumpStreamType, MinidumpLocationDescriptor>`?
> > 
> > No point erasing the type information of the enum.
> If I use MinidumpStreamType as the keys type then I think I have to specify the getEmptyKey() and getTombstoneKey() functions via DenseMapInfo.
> And in the "unsigned" template specialisation of DenseMapInfo:
> 
> 
> ```
> // Provide DenseMapInfo for unsigned ints.
> template<> struct DenseMapInfo<unsigned> {
>   static inline unsigned getEmptyKey() { return ~0U; }
>   static inline unsigned getTombstoneKey() { return ~0U - 1; }
>   static unsigned getHashValue(const unsigned& Val) { return Val * 37U; }
>   static bool isEqual(const unsigned& LHS, const unsigned& RHS) {
>     return LHS == RHS;
>   }
> };
> ```
> 
> So I should probably add there "special" values in the enum as well in order for it to work?
That's unfortunate, but it looks like you're right.  It's probably not worth going to that much effort.  It could probably be done by partially specializing `DenseMapInfo` for all enums, but I don't think it's worth it.  So I suppose it's fine to leave as a `uint32_t`.


https://reviews.llvm.org/D23545