[PATCH] D66418: [llvm-objdump] - Cleanup the error reporting.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:381
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, Twine Message) {
-  WithColor::error(errs(), ToolName)
-      << "'" << File << "': " << Message << ".\n";
+  if (File.empty())
+    WithColor::error(errs(), ToolName) << Message << ".\n";
----------------
MaskRay wrote:
> Keeping a separate `LLVM_ATTRIBUTE_NORETURN void report_error(Twine Message);` overload for the error message without a filename might be clearer. What do you think?
I had to add this code to `report_error` and `report_warn` because of 2 users:

```
    report_warn("section '" + S +
                    "' mentioned in a -j/--section option, but not "
                    "found in any input file",
                "" /*FileName*/);
```

and

```
report_error("" /*FileName*/, "start address should be less than stop address");
```

I.e. these are kind of exceptions, as others calls always have `FileName`.

What I afraid here is that people might want to continue using the `void report_error(Twine Message)`
everywhere if we have one. I am not sure what is better. Lets see what others think may be.

May be we should print something like the following for these two ?

```
warn: -j/--section: section XXX mentioned, but not found in any input file

error: --start-address/--stop-address: start address should be less than stop address
```

Then we do not need to have any code for an empty file name.


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

https://reviews.llvm.org/D66418