[PATCH] D81541: [ObjectYAML][DWARF] Implement the .debug_addr section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-addr.yaml:243
+      SegmentSelectorSize: 0x00
+      Entries:             []
+
----------------
Higuoxing wrote:
> jhenderson wrote:
> > You need a test case each for missing the version and address size fields.
> `Version` is a required field, do we still need to test it here?
Right, but how do you show that `Version` is required? You need a test showing that it is, i.e. that you get an error message if the `Version` is unspecified.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-addr.yaml:155
+        - Address: 0x5678
+    ## 2) Only address (8-byte) exists.
+    - Version: 5
----------------
This comment is a bit stale, since on first glance, it looked like "are you missing the address?" before I realised that it's derived by default.

It might be best to update this comment to say something like "Show that the address size is derived from the file header." possibly in addition to other things, as you think are best. Same sort of comment below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81541