[PATCH] D87429: [obj2yaml] Add support for dumping the .debug_ranges section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml:7
+# RUN: yaml2obj --docnum=1 -DBITS=32 -DLOWOFFSET=0xffffffff -DHIGHOFFSET=0x10 %s | \
+# RUN:   obj2yaml | FileCheck -DADDRSIZE=0x04 -D#%x,OFFSET=0x18 -D#%x,LOWOFFSET=0xffffffff -D#%x,HIGHOFFSET=0x10  %s --check-prefix=BASIC
+
----------------
1) See below re. numeric versus string variables.
2) Reflow the FileCheck command - it's way too long for one line.
3) I recommend reordering the commands this way, since the first bits don't change at all.
4) There was a sneaky double space before `%s`.

Same applies throughout.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml:46
+# BASIC-NEXT:         HighOffset: 0x0000000000000040
+# BASIC-NEXT:   - Offset:   0x[[#%.16X,OFFSET]]
+# BASIC-NEXT:     AddrSize: [[ADDRSIZE]]
----------------
As you're not doing any arithmetic here, I'd just match the number as a string. Same below with the low/high offsets. This simplifies both the check here and the definition above (i.e. `-DOFFSET=0x18` or whatever).


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml:58-62
+  debug_info:
+    - Version:  4
+      AddrSize: [[ADDRSIZE1=<none>]]
+    - Version:  4
+      AddrSize: [[ADDRSIZE2=<none>]]
----------------
Add a comment explaining the need for the debug_info bit.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml:85
+##                           ^--------------- LowOffset
+#      RAW:                  2000000000000000
+##                           ^--------------- HighOffset
----------------
This and the following lines probably want to be RAW-SAME, and start with a `{{$}}` to ensure there's nothing in between the previous match and the checked-for one.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml:108
+
+#      EMPTY: DWARF:
+# EMPTY-NEXT:   debug_ranges: []
----------------
Probably should show that there's no `Sections:` tag here.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-ranges.yaml:122
+## Override the sh_type field.
+# RUN: yaml2obj --docnum=3 -DTYPE=SHT_STRTAB %s | obj2yaml | FileCheck %s -DTYPE=STRTAB --check-prefixes=RANGES,SHDR
+
----------------
Same below. Whilst the 80-column width style guide doesn't really apply in tests, it's good to avoid the lines getting too long, in my opinion, as it is easier to read.

Also RANGES and SHDR are always both checked, so combine them to a single prefix (e.g. `COMMON`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87429