[PATCH] D77146: [DebugInfo] Fix reading location tables headers of v5 units in DWP.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:541-542
       Offset -= HeaderSize;
+      if (auto *IndexEntry = Header.getIndexEntry())
+        if (const auto *C = IndexEntry->getOffset(DW_SECT_LOCLISTS))
+          Offset += C->Offset;
----------------
jhenderson wrote:
> ikudrin wrote:
> > jhenderson wrote:
> > > Maybe these shouldn't be `auto`? Certainly, `C` looks like it should be a number (e.g. uint64_t), but it obviously isn't.
> > `getOffset()` returns `const DWARFUnitIndex::Entry::SectionContribution *`. Not that obvious, I agree. Do you think it would be better to rename `C` to, say, `Contrib` or `Contribution`?
> I think the issue actually is the function name: `getOffset` probably wants renaming to something like `getContribution`. `Contrib` is probably fine for the variable name (though I'm not sure if using`auto` here isn't a violation of LLVM's coding standards, regardless of the variable or function name).
Yeah, I don't mind the rename - either before or after this change. I'm pretty OK with "auto" given the very short scope & pointer-ness (making it obvious what boolean testing the thing does, etc)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77146/new/

https://reviews.llvm.org/D77146