[PATCH] D65255: [yaml2obj] Move core yaml2obj code into lib and include for use in unittests

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 26 04:54:23 PDT 2019


jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/CMakeLists.txt:1
 add_llvm_library(LLVMObjectYAML
   CodeViewYAMLDebugSections.cpp
----------------
abrachet wrote:
> jhenderson wrote:
> > Looks like this CMake is missing any reference to the include files, which means they won't get auto-magically added to things like Visual Studio's projects. If you take a look at libObject's CMakeLists.txt, you'll find an example of what to do.
> CMake doesn't fail when generating files so I haven't done anything horribly wrong. I assume this works but I don't know how to test if it did.
I checked with equivalent code, and it did the job on my Windows machine.


================
Comment at: llvm/lib/ObjectYAML/yaml2obj.cpp:20
+Error convertYAML(yaml::Input &YIn, raw_ostream &Out, unsigned DocNum) {
+  // Error with empty string. In the future the Error will be propagated from
+  // yaml2* functions, it doesn't make sense to add to the output here.
----------------
Maybe mark this with TODO:

Rather than an empty string, let's at least report something like "yaml2obj failed".


================
Comment at: llvm/lib/ObjectYAML/yaml2obj.cpp:22
+  // yaml2* functions, it doesn't make sense to add to the output here.
+  auto IntToErr = [](int Ret) -> Error {
+    if (Ret)
----------------
You don't need the trailing return type, I'm pretty sure.


================
Comment at: llvm/test/tools/yaml2obj/invalid-docnum.test:23
+  Machine: EM_X86_64
\ No newline at end of file

----------------
Nit: no new line at end of file.


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

https://reviews.llvm.org/D65255





More information about the llvm-commits mailing list