[PATCH] D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers.

Wed Sep 11 08:16:53 PDT 2019

sbc100 added inline comments.

================
Comment at: include/llvm/ObjectYAML/yaml2obj.h:49

-Error convertYAML(Input &YIn, raw_ostream &Out, unsigned DocNum = 1);
+bool yaml2coff(COFFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH);
+bool yaml2elf(ELFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH);
----------------
grimar wrote:
> sbc100 wrote:
> > It seems like there is a separate change here which is to convert a lot of return codes from "int -> 0 for success" to "bool -> 1 for success".  Is that objectively better?    Is there an llvm policy for which is preferred?   Could this change be more precise if that part was split out? 
> I believe it is better because we only use `1` and `0`. There are no another values => the valid type to represent it should be `bool`. I never heard about policies about the returning types in the libraries. And it is a bit hard to imagine one that would say to use `int` when we can use `bool` :)
> 
> I am OK to split it out I think, I am not sure if it can make this diff much shorter/easier to read, but I'll try.
I think its mostly a split between the old school C-style APIs where zero means success and non-zero means failure.    I'm not sure how relevant this is in modern C++ codebases though..   perhaps not at all.

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

https://reviews.llvm.org/D67445