[PATCH] D64800: [llvm-readobj] - Stop using precompiled objects in file-headers.test

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 19 05:01:00 PDT 2019


jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/coff-file-headers.test:5
+
+# ARM:File: [[FILE]]
+# ARM-NEXT:Format: COFF-ARM
----------------
Up to you what you think, but you can add some leading spaces after the '#' to keep things lining up nicely:

```
#      ARM:File:
# ARM-NEXT:Arch:
```


================
Comment at: test/tools/llvm-readobj/coff-file-headers.test:33
+# ARM64:File: [[FILE]]
+# ARM64-NEXT:Format: COFF-ARM64
+# ARM64-NEXT:Arch: aarch64
----------------
grimar wrote:
> MaskRay wrote:
> > Due to `--strict-whitespace`, there are now no space after `NEXT:`. Just a question: do you find anything that requires `--strict-whitespace`?
> In this patch we are testing how the headers are dumped.
> And this is the main and the only test set atm for headers output I think. 
> I supposed that we might want to check that do not print excessive spaces for example somewhere.
> My feeling is not strong here though. I am OK to remove `--strict-whitespace` to make the tests nicer probably.
> 
> 
I prefer having --strict-whitespace like you've got here. This helps us catch odd formatting issues that sometimes creep in. We don't need it on every test case, but for the basic ones, it makes sense.


================
Comment at: test/tools/llvm-readobj/coff-file-headers.test:57
+# RUN: llvm-readobj -h %t.i386 \
+# RUN:  | FileCheck %s --strict-whitespace --match-full-lines -DFILE=%t.i386 -check-prefix I386
+
----------------
Nit: use double dash for check-prefix here and elsewhere, to be consistent with other options.


================
Comment at: test/tools/llvm-readobj/coff-file-headers.test:322
+# IMPORTLIB-NOT:{{.}}
\ No newline at end of file

----------------
Nit missing new line at EOF.


================
Comment at: test/tools/llvm-readobj/elf-file-headers.test:130
+# LANAI-NOT:{{.}}
\ No newline at end of file

----------------
Nit: no new line at EOF.


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

https://reviews.llvm.org/D64800





More information about the llvm-commits mailing list