[PATCH] D26514: Get rid of DWARFFormValue::getFixedFormSizes(uint8_t AddrSize, uint16_t Version).

[Adrian Prantl via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFUnit.h:195
+  llvm::dwarf::DwarfFormat getFormat() const {
+    return llvm::dwarf::DwarfFormat::Dwarf32; // FIXME: Support DWARF64.
+  }
----------------
I think the llvm:: namespace is redundant here.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFUnit.h:207
+  uint8_t getDwarfOffsetByteSize() const {
+    if (getFormat() == llvm::dwarf::DwarfFormat::Dwarf32)
+      return 4;
----------------
Up to taste, maybe write this as:

```
switch (getFormat()) {
  case dwarf::DwarfFormat::Dwarf32: return 4;
  case dwarf::DwarfFormat::Dwarf64: return 8;
}
```


================
Comment at: include/llvm/Support/Dwarf.h:438
+enum DwarfFormat {
+  Dwarf32,
+  Dwarf64
----------------
Maybe call this DWARF32 and DWARF64 since that is the correct spelling?


================
Comment at: lib/DebugInfo/DWARF/DWARFFormValue.cpp:363
+                             const DataExtractor &debug_info_data,
+                             uint32_t *offset_ptr, const DWARFUnit *U) {
   bool indirect = false;
----------------
Should we make this `const DWARFUnit *U = nullptr` since you're using it that way later on?


https://reviews.llvm.org/D26514