[PATCH] D67182: [lib/ObjectYAML] - Improve and cleanup error reporting in ELFState<ELFT> class.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.

LG once you remove the redundant `-o %t2` in a few places.



================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:117
 
-  bool buildSectionIndex();
-  bool buildSymbolIndexes();
+  bool HasError = false;
+
----------------
grimar wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > Instead of `HasError`, have you thought using an integer (`int errorCount = 0;`) ?
> > Sorry, `int ErrorCount = 0;`
> > 
> > Is there a case that many errors may be accumulated? If yes, we may need an ErrorLimit. We certainly don't need `ErrorLimit` in this patch, but using `ErrorCount` may prevent a future rename `HasError -> ErrorCount`..
> I'd do this when we need it. It is not clear how much it is useful, since it is not the same case as we have in linker: I do not think there can be many errors in a hand written YAML document. And probably no need to limit them.
> obj2yaml is assumed to produce only valid YAMLs too.
I was just checking whether we'll eventually do that. So using the final variable name can prevent a future rename. You make the call. I haven't put much thought on that, either.


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

https://reviews.llvm.org/D67182