This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Partial support of --gdb-index command line option (Part 2).
ClosedPublic

Authored by grimar on Oct 20 2016, 4:58 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 75286.Oct 20 2016, 4:58 AM
grimar retitled this revision from to [ELF] - Partial support of --gdb-index command line option (Part 2)..
grimar updated this object.
grimar added subscribers: llvm-commits, grimar, evgeny777.
grimar added a subscriber: davide.Oct 20 2016, 4:59 AM
grimar updated this revision to Diff 76542.Nov 1 2016, 5:16 AM
  • Reimplemented using LoadedObjectInfo interface. It removes need of D25822.
grimar updated this revision to Diff 80758.Dec 8 2016, 7:05 AM

Ping.

  • Rebased.
aprantl accepted this revision.Dec 14 2016, 9:01 AM
aprantl edited edge metadata.

Small comments inline.

ELF/GdbIndex.h
23 ↗(On Diff #80758)

These comments are not useful, they just repeat the name of the fields. Can you instead add a doxygen comment that describes the purpose of the struct on top?

This revision is now accepted and ready to land.Dec 14 2016, 9:01 AM
aprantl added inline comments.Dec 14 2016, 9:03 AM
ELF/GdbIndex.h
29 ↗(On Diff #80758)

Comment what this is?

38 ↗(On Diff #80758)

Please add Doxygen comments too all these functions.

39 ↗(On Diff #80758)

Up to taste, but I think I would spell this readCUList.

This revision was automatically updated to reflect the committed changes.
grimar added inline comments.Dec 15 2016, 1:19 AM
ELF/GdbIndex.h
23 ↗(On Diff #80758)

Done.

29 ↗(On Diff #80758)

Done.

38 ↗(On Diff #80758)

Done.

39 ↗(On Diff #80758)

Done.