[PATCH] D65313: [llvm-readelf] Dump the stack sizes sections with --stack-sizes

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 2 00:57:47 PDT 2019


grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4403
+    } else if (*SectionOrErr != FunctionSec) {
+      reportWarning(" '" + FileStr + "': relocation symbol " + SymName +
+                    " is not in the expected section");
----------------
3 comments/suggestions:

1) Instead of manually constructing the message with a file name,
    you can use the same approach as already used now for reporting a errors.
i.e. just introduce a method:

```
void reportWarning(StringRef Input, Error Err) {
  if (Input == "-")
    Input = "<stdin>";
  warn(createFileError(Input, std::move(Err)));
}
```
(see my D65515 for reference).

2) With that you do not need to call `consumeError` after `reportWarning`.

3) I think everywhere where you used `consumeError` you should probably report a warning.
Like in the code above:

```
    if (NameOrErr)
      SymName = *NameOrErr;
    else
      consumeError(NameOrErr.takeError());
```
i.e. call `reportWarning` from (1) instead.

What do you think?


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

https://reviews.llvm.org/D65313





More information about the llvm-commits mailing list