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

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 7 inline comments as done.
abrachet added inline comments.


================
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)
----------------
jhenderson wrote:
> You don't need the trailing return type, I'm pretty sure.
For the first time, this one actually needs it :) `Error::success()` returns an `ErrorSuccess`


================
Comment at: llvm/tools/yaml2obj/yaml2obj.cpp:45
 
-static int convertYAML(yaml::Input &YIn, raw_ostream &Out) {
-  unsigned CurDocNum = 0;
----------------
labath wrote:
> abrachet wrote:
> > 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.
> I'm pretty ambivalent. On one hand, it's nice to have the same interface as the command line tool, but on the other, in the test you should always know the object type, so appending that to the function name does not hurt.
I think the alternative would be code used in the same way in every test which wants to use it. See D64281 for how it can be used in unit tests. That was the same pattern that @labath did here D59482. It's not just about explicitly stating the object file type, this also does error handling etc.


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

https://reviews.llvm.org/D65255