[PATCH] D58194: [DebugInfo] add SectionedAddress to DebugInfo interfaces.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 14 00:04:37 PST 2019

grimar added a comment.

My comments about LLD side are below.

Question is: can LLD part have a test?
I also wonder could this patch just pass `object::SectionedAddress::UndefSection`,
would behavior be the same? If so, then probably would be better to split LLD change
to a different patch with the test case.

Comment at: lld/ELF/InputFiles.cpp:208
+  // detect SectionIndex for specified section.
+  // TODO : put section index information into InputSectionBase itself
detect -> Detect

(comments should start from uppercase)

Comment at: lld/ELF/InputFiles.cpp:210
+  // TODO : put section index information into InputSectionBase itself
+  // to not loop over all sections.
+  uint64_t SectionIndex = object::SectionedAddress::UndefSection;
I would like to hear what Rui thinks about this,
but my opinion this TODO should be removed because I do not believe we will
add a section index to `InputSectionBase`. This method is used for error reporting,
so it is OK for it to be reasonably slow. But adding bits to `InputSectionBase` might increase
memory consumption.

And FTR I do not know a better way to find the section index here, seems
using the loop is fine.

Comment at: lld/ELF/InputFiles.cpp:213
+  ArrayRef<InputSectionBase *> Sections = S->File->getSections();
+  for (uint64_t CurIndex = 0; CurIndex < Sections.size(); CurIndex++) {
+    if (S == Sections[CurIndex]) {
We usually use prefix increments I think:




More information about the llvm-commits mailing list