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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to