[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