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

[George Rimar via Phabricator via llvm-commits]

grimar marked 3 inline comments as done.
grimar added inline comments.


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:117
 
-  bool buildSectionIndex();
-  bool buildSymbolIndexes();
+  bool HasError = false;
+
----------------
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.


================
Comment at: test/tools/yaml2obj/dynamic-section-raw-content.yaml:29
 
+# RUN: not yaml2obj --docnum=2 %s -o %t2 2>&1 | FileCheck %s --check-prefix=ERR
+
----------------
MaskRay wrote:
> Can `-o %t2` be deleted?
Yes, you're right here and below. I'll update the diff in my next week or tomorrow.


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

https://reviews.llvm.org/D67182