[PATCH] D59946: [llvm-readobj] Improve error message for for --string-dump

[James Henderson via Phabricator via llvm-commits]

jhenderson added reviewers: rupprecht, grimar, Higuoxing.
jhenderson added a comment.

I've added a few more reviewers who have been reviewing stuff in this area recently. It would be good to get some other comments on the interface changes with regards to error message printing.



================
Comment at: llvm/test/tools/llvm-readobj/elf-dynamic-malformed.test:6
 
-# ERR-SIZE: Error reading file: Invalid entity size.
+# ERR-SIZE: {{.+}} Invalid entity size
 
----------------
I assume this is testing the file name? If so could you do similar to what is done above, e.g. in basic.test.


================
Comment at: llvm/tools/llvm-readobj/ObjDumper.cpp:58-59
   }
-  return make_error<StringError>("invalid section reference",
-                                 object::object_error::parse_failed);
+  return make_error<StringError>(
+      formatv("could not find section '{0}'", SecName),
+      object::object_error::parse_failed);
----------------
I think you'd be better off using createStringError() here, which takes variadic arguments and formats in printf style.


================
Comment at: llvm/tools/llvm-readobj/llvm-readobj.cpp:405-406
 
-static void reportError(StringRef Input, Error Err) {
-  if (Input == "-")
-    Input = "<stdin>";
-  std::string ErrMsg;
-  {
-    raw_string_ostream ErrStream(ErrMsg);
-    logAllUnhandledErrors(std::move(Err), ErrStream, Input + ": ");
-  }
-  reportError(ErrMsg);
+static void reportError(StringRef Input, std::error_code EC) {
+  reportError(Input, errorCodeToError(EC));
 }
----------------
Nit: Could you move this above the reportError immediately above, so that it lines up well in diffs with the old version, please?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59946