[PATCH] D74197: [DebugInfo] Simplify DWARFDebugAddr.

Fri Feb 7 17:35:21 PST 2020

ikudrin marked an inline comment as done.
ikudrin added inline comments.

================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h:65
+  /// a header and consists only of a series of addresses.
+  Error extractPreV5(const DWARFDataExtractor &Data, uint64_t *OffsetPtr,
+                     uint16_t CUVersion, uint8_t CUAddrSize);
----------------
dblaikie wrote:
> aprantl wrote:
> > Assuming that DWARF 6 doesn't change the format again, this naming scheme will look odd in the future.
> > 
> > How about we call this extractV2() and mention in the comment it is for 2 through 4? I.e., always use the minimum version that introduced the encoding in the name?
> Agreed - generally the different sections don't rev their versions until their own format changes (eg: debug_line still used v2 even though debug_info was v3 or v4) so I think it makes sense to name them as you're suggesting (with a comment about the valid range - "used up until DWARFv4", "used until at least DWARFv5(current)" or whatever seems good)
If DWARF6 would not change the format of this section, they probably keep the value of the version field, so the function name will still sound accurate as "extract an address table in the format of version 5 (of address table)".

As for "PreV5", the section and its usage were defined only in DWARF5. Before that, there was only a [[ https://gcc.gnu.org/wiki/DebugFission | proposal from GCC ]]. So the name of the function should be read as "extract an address table as it was defined (in some other document) before DWARF5".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74197