[PATCH] D24267: [ELF] - Added support of --gdb-index command line option.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 6 14:38:28 PDT 2016


ruiu added a comment.

It is a very large patch that has almost no comment, so it is hard to read (I didn't read through till end.) Non-code part is as important as code. It really needs overview comments so that first-time readers of the code like me can get an idea what you are trying to do with the code and what your intention is. What is the expected output, and what kinds of inputs are used to create the desired output? Why do we need such output in the first place? You want to explain these kind of things in this patch like writing a blog article about this feature.


================
Comment at: ELF/OutputSections.cpp:65
@@ +64,3 @@
+template <class ELFT>
+void GdbIndexSection<ELFT>::parceDebugInputSections(
+    OutputSectionBase<ELFT> *Sec) {
----------------
parse

================
Comment at: ELF/SplitDebugInfo.h:1
@@ +1,2 @@
+//===- ScriptParser.h -------------------------------------------*- C++ -*-===//
+//
----------------
Fix the comment.

================
Comment at: ELF/SplitDebugInfo.h:180
@@ +179,3 @@
+public:
+  std::pair<bool, GdbSymbol *> add(uint32_t Hash, size_t Offset) {
+    if (Size * 4 / 3 >= Table.size())
----------------
Please move definitions to the .cpp file.


https://reviews.llvm.org/D24267





More information about the llvm-commits mailing list