[PATCH] D83131: [llvm-readobj] - Refine the error reporting in LLVMStyle<ELFT>::printELFLinkerOptions.

Mon Jul 6 12:53:25 PDT 2020

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

In D83131#2131493 <https://reviews.llvm.org/D83131#2131493>, @MaskRay wrote:

> > 2. replace reportWarning with reportUniqueWarning calls. In this method it is no-op, because it is not possible to have a duplicated warnings anyways, but since we probably want to switch to reportUniqueWarning globally, this is a good thing to do.
>
> My understanding of the inline comments in D69671 <https://reviews.llvm.org/D69671> is that we will change `reportWarning` call sites to use `reportUniqueWarning` and then rename `reportUniqueWarning` to `reportWarning`. Is that the case?

Yes.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/linker-options.test:12
 # CHECK-NEXT:   warning: '[[FILE]]': SHT_LLVM_LINKER_OPTIONS section at index 4 is broken: the content is not null-terminated
+# CHECK-NEXT:   warning: '[[FILE]]': unable to read the content of the SHT_LLVM_LINKER_OPTIONS section at index 5: section [index 5] has a sh_offset (0xffffffff) + sh_size (0x8) that is greater than the file size (0x370)
 # CHECK-NEXT:   option 3: value 3
----------------
jhenderson wrote:
> Repeating the "index 5" bit in the warning seems sub-optimal. I think it's only necessary if we don't trust the warning produced by the Object library to include the index?
That is why I've introduced it, but now I see that `getSectionContents()` always includes the index and it is probably unlikely that one day it will get to some another case when it will not do it. I'll update this place.

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

https://reviews.llvm.org/D83131