[PATCH] D80203: [ObjectYAML][DWARF] Add DWARF entry in ELFYAML.

Fri May 29 01:35:56 PDT 2020

jhenderson added a comment.

It looks like my last batch of test comments weren't addressed. Please could you post a new patch to fix them all.

================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:145
+    Offset:       0x00000050         # 0x40 for the first section.
+    Size:         6                  # Set the "Size" so that we can reuse the check tag "OVERRIEDDEN"
+
----------------
jhenderson wrote:
> There's a typo: OVERRIEDDEN -> OVERRIDDEN
It looks like this comment wasn't addressed?

================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:192
+
+## i) Test that yaml2obj will generate the .debug_str section whose sh_type is a preserved one,
+## e.g. SHT_DYNAMIC
----------------
jhenderson wrote:
> I'm not sure "preserved" makes sense here. Probably you mean "is a type which usually uses a different syntax".
This comment wasn't addressed either. "preserved" is not the right word for this sentence ("preserve" means "to keep something from going bad" or "to protect" and similar meanings).

"with a preserved "Type"" -> "with a "Type" with non-standard syntax"

================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-str.yaml:198
+
+#                 Name      Type    Address          Offset Size   ES Flg Lk Inf Al
+# DYNAMIC-EMPTY: .debug_str DYNAMIC 0000000000000000 000040 000000 01 MS  0  0   0
----------------
jhenderson wrote:
> Did you deliberately not include the DYNAMIC-EMPTY tag here? If so, you need '##'. Same applies in all the other test cases too.
This comment wasn't addressed either.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80203