[PATCH] D25821: [ELF] - Partial support of --gdb-index command line option (Part 2).
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 15 01:19:02 PST 2016
grimar added inline comments.
================
Comment at: ELF/GdbIndex.h:23
+template <class ELFT> struct AddressEntry {
+ InputSectionBase<ELFT> *Section; // Section.
+ uint64_t LowAddress; // The low address.
----------------
aprantl wrote:
> 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?
Done.
================
Comment at: ELF/GdbIndex.h:29
+
+template <class ELFT> class GdbIndexBuilder : public LoadedObjectInfo {
+ typedef typename ELFT::uint uintX_t;
----------------
aprantl wrote:
> Comment what this is?
Done.
================
Comment at: ELF/GdbIndex.h:38
+ GdbIndexBuilder(InputSection<ELFT> *DebugInfoSec);
+
+ std::vector<std::pair<uintX_t, uintX_t>> readCuList();
----------------
aprantl wrote:
> Please add Doxygen comments too all these functions.
Done.
================
Comment at: ELF/GdbIndex.h:39
+
+ std::vector<std::pair<uintX_t, uintX_t>> readCuList();
+ std::vector<AddressEntry<ELFT>> readAddressArea(size_t CurrentCU);
----------------
aprantl wrote:
> Up to taste, but I think I would spell this `readCUList`.
Done.
Repository:
rL LLVM
https://reviews.llvm.org/D25821
More information about the llvm-commits
mailing list