This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readelf/obj] - Deduplicate the logic that prints notes. NFCI.
ClosedPublic

Authored by grimar on Nov 24 2020, 3:08 AM.

Details

Summary

We have a similar logic for LLVM/GNU styles that can be deduplicated.
This will allow to replace reportError calls with reportUniqueWarning
calls in a single place.

Diff Detail

Event Timeline

grimar created this revision.Nov 24 2020, 3:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2020, 3:08 AM
Herald added a subscriber: rupprecht. · View Herald Transcript
grimar requested review of this revision.Nov 24 2020, 3:08 AM
jhenderson added inline comments.Nov 24 2020, 5:44 AM
llvm/tools/llvm-readobj/ELFDumper.cpp
6847

I might be missing something, but I don't see this scoping in the old behaviour? Without it, we also wouldn't need the EndNotes heper.

6847

Never mind... spotted it. Will look a bit more.

jhenderson accepted this revision.Nov 24 2020, 5:49 AM

Looks okay. I'm not a massive fan of having three lambdas though, and I wonder whether they'd be better off as virtual functions, at least to some extent, of the dump style classes (probably only for the ProcessNote one though, I accept).

This revision is now accepted and ready to land.Nov 24 2020, 5:49 AM
MaskRay accepted this revision.Nov 24 2020, 3:20 PM
nlguillemot added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
5562

This line gives a warning with clang's "range-loop analysis" diagnostic:

error: loop variable 'Note' is always a copy because the range of type 'iterator_range<llvm::object::ELFFile<llvm::object::ELFType<llvm::support::big, true> >::Elf_Note_Iterator>' (aka 'iterator_range<Elf_Note_Iterator_Impl<ELFType<(llvm::support::endianness)0U, true> > >') does not return a reference [-Werror,-Wrange-loop-analysis]
      for (const typename ELFT::Note &Note : Obj.notes(S, Err))

It can be fixed by removing the ampersand.

grimar marked an inline comment as done.Nov 25 2020, 11:25 PM
grimar added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
5562