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

[Dimitar Vlahovski via lldb-commits]

dvlahovski 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;
+};
----------------
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?

================
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:133
@@ +132,3 @@
+// This matches the Linux kernel definitions from <asm/hwcaps.h>
+enum MinidumpPCPUInformationARMElfHwCaps
+{
----------------
zturner wrote:
> Should this be `enum class`?
Yes, forgot to change it ...


https://reviews.llvm.org/D23545