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

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 9 inline comments as done.
abrachet added a comment.

`git clang-format` does not understand moved files it turns out, so it formatted things that I didn't touch. I figure if this is going to happen it might as well be now, though. I can change this back though.



================
Comment at: llvm/lib/ObjectYAML/CMakeLists.txt:1
 add_llvm_library(LLVMObjectYAML
   CodeViewYAMLDebugSections.cpp
----------------
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.


================
Comment at: llvm/lib/ObjectYAML/yaml2wasm.cpp:655
 
-int yaml2wasm(llvm::WasmYAML::Object &Doc, raw_ostream &Out) {
+int llvm::yaml::yaml2wasm(WasmYAML::Object &Doc, raw_ostream &Out) {
   WasmWriter Writer(Doc);
----------------
MaskRay wrote:
> It is probably time to wrap these things in
> 
> ```
> namespace llvm {
> namespace yaml {
> ...
> }
> }
> ```
I did this in all the files. Also I wrapped things in an anonymous namespace, is this ok?


================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:45
 
-static int convertYAML(yaml::Input &YIn, raw_ostream &Out) {
-  unsigned CurDocNum = 0;
----------------
MaskRay wrote:
> There is probably not much we gain by moving this part to lib/ObjectYAML.
I think its more convenient to not have to specify the file type. I wonder what others have to say.


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

https://reviews.llvm.org/D65255