[PATCH] D129713: [obj2yaml] Add -o to specify output filename

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 13 23:51:25 PDT 2022


jhenderson added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/Archives/regular.yaml:8-10
+## -o specifies an output file.
+# RUN: obj2yaml %t.empty.a -o %t.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file=%t.yaml %s --check-prefix=EMPTY
----------------
I wonder if we could avoid some test duplication in these test cases by doing the following pattern instead:
```
# RUN: obj2yaml %t.empty.a > %t.stdout.yaml
# RUN: obj2yaml %t.empty.a -o %t.file.yaml 2>&1 | count 0
# RUN: FileCheck --input-file=%t.stdout.yaml %s --check-prefix=EMPTY
# RUN: diff %t.stdout.yaml %t.file.yaml
```

What do you think?

The same sort of pattern could apply to the other test cases too.


================
Comment at: llvm/tools/obj2yaml/obj2yaml.cpp:64
   case file_magic::offload_binary:
-    return offload2yaml(outs(), MemBuf);
+    return offload2yaml(OS, MemBuf);
   default:
----------------
I guess there isn't an appropriate offloading test case you could update like you have with the others?


================
Comment at: llvm/tools/obj2yaml/obj2yaml.cpp:114
   }
+  Out->keep();
 
----------------
Should we have a test case that if there was an error, the output file is not kept? It seems like a simple extension to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129713



More information about the llvm-commits mailing list