[PATCH] D92090: [llvm-readelf/obj] - Stop calling `reportError` in `printArchSpecificInfo()`.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 26 01:57:06 PST 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/mips-got.test:379-380
+# ERR1-NEXT: warning: '[[FILE]]': cannot find PLTGOT dynamic tag
+# ERR1-EMPTY:
+# ERR1-NOT: {{.}}
 
----------------
jhenderson wrote:
> Entirely optional, but if you were to replace the second and third line prefixes of this and each subsequent test case with NO-OUTPUT, and then put it at the end of the file, you wouldn't need to repeat the `-EMPTY`, `-NOT` lines for each test case. If you did that, I might reorder some things, roughly like this:
> 
> ```
> # RUN: <existing ERR1 run line(s)>
> 
> # RUN: <existing ERR2 run line(s)>
> 
> ...
> 
> # NO-OUTPUT:      LoadName: <Not found>
> # NO-OUTPUT-NEXT: There is no .MIPS.abiflags section in the file.
> # NO-OUTPUT-NEXT: There is no .MIPS.options section in the file.
> # NO-OUTPUT-NEXT: There is no .reginfo section in the file.
> 
> # ERR1-NEXT: warning: '[[FILE]]': cannot find PLTGOT dynamic tag
> # ERR2: warning: '[[FILE]]': cannot find MIPS_LOCAL_GOTNO dynamic tag
> ...
> 
> # NO-OUTPUT-EMPTY:
> # NO-OUTPUT-NOT: {{.}}
> ```
> 
> Same comment goes for the other test file.
Done. I thought about something like this, but without grouping "ERR" check lines together (i.e. only supposed that we
can move the last 2 "NO-OUTPUT-*" lines later. Was not sure about this solution.

I think that your version where all check lines are grouped together in one place looks good.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/mips-got.test:385
 
-# ERR2: error: '[[FILE]]': cannot find MIPS_LOCAL_GOTNO dynamic tag
+# ERR2: warning: '[[FILE]]': cannot find MIPS_LOCAL_GOTNO dynamic tag
+# ERR2-EMPTY:
----------------
jhenderson wrote:
> Is there output between the end of the NO-OUTPUT block and ERR2 (also ERR3, ERR4 etc)? The ERR1 case uses `-NEXT` here.
Used `-NEXT`.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/mips-plt.test:99
+
+# ERR1: warning: '[[FILE]]': cannot find JMPREL dynamic tag
+# ERR1-EMPTY:
----------------
jhenderson wrote:
> As above, can this and below use -NEXT?
Done.


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

https://reviews.llvm.org/D92090



More information about the llvm-commits mailing list