[PATCH] D42481: [DebugInfo] Add basic support for DWARF 5 .debug_rnglists dumping

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugRnglists.h:23
+public:
+  struct BoundedRange {
+    uint64_t Start;
----------------
JDevlieghere wrote:
> As this is so similar to `DWARFAddressRange` it would be nice if we could reuse that in some way.  We already have infrastructure to verify these entries for things like overlap that we might want to reuse in the verifier. 
Good idea. I wonder if DWARFAddressRange should be moved into its own file, since it's not specific to the DWARFDebugRangeList class?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:488
+                                   isLittleEndian(), 0);
+    DWARFDebugRnglists Set;
+    uint32_t Offset = 0;
----------------
JDevlieghere wrote:
> Set?
Oops, that was leftover from an earlier version when the class was called something to do with sets.


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp:23
+
+bool DWARFDebugRnglists::extract(DWARFDataExtractor Data, uint32_t *OffsetPtr) {
+  clear();
----------------
JDevlieghere wrote:
> Let's return an LLVM error instead of a bool so we can convey a useful error message to the caller.
I like this idea. It allows for more fine-grained behaviour than simply not printing the section.


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRnglists.cpp:117
+  const uint32_t HexWidth = HeaderData.AddrSize * 2;
+  for (const auto &List : Ranges) {
+    for (const auto &Entry : List) {
----------------
JDevlieghere wrote:
> Same here and on the next line.
There are two lines in the outer loop!


Repository:
  rL LLVM

https://reviews.llvm.org/D42481