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

[George Rimar via Phabricator via llvm-commits]

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:

```
++CurIndex
```



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58194/new/

https://reviews.llvm.org/D58194