[PATCH] D89949: [yaml2obj][obj2yaml] - Teach tools to work with regular archives.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:1
+## Check how obj2yaml dumps regular archives.
+
----------------
Possible additional test case: dumping the output when the header fields are empty, where such are "legal" (i.e. I think all but the name).


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:59
+    PaddingByte:  0x0A
+## All fields are explicitly set to values by default.
+  - Name:         ''
----------------
I think this comment better explains what's going on?


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:94
+
+## Check we report an error when unable to read the data of of an archive member.
+
----------------



================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:104
+Members:
+  - Size: [[SIZE='']]
+
----------------
Looks like you could use `'1'` as the default here?


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:106
+
+## Check we report an error when unable to read the size of of an archive member.
+
----------------



================
Comment at: llvm/tools/obj2yaml/archive2yaml.cpp:37
+        return createStringError(std::errc::illegal_byte_sequence,
+                                 "unable to read the header of a child");
+
----------------
I think it might be useful to include the offset of the bad header here. What do you think?

Same comment probably applies for most other messages below.


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

https://reviews.llvm.org/D89949