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