[PATCH] D73047: [ELF] Refactor uses of getInputSections to improve efficiency NFC

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Not got any objections to the refactoring, I'm not sure we need to share the loop traversal between getInputSections and getFirstInputSections (see comment inline) but I'm willing to go with the consensus on that.



================
Comment at: lld/ELF/OutputSections.cpp:469
+namespace {
+void forEachInputSectionDescription(
+    const OutputSection *os, function_ref<bool(InputSectionDescription *)> f) {
----------------
It would definitely be worth adding a comment, explaining that f must return true to continue and false to early terminate. I didn't find it obvious until reading the loop body and the two examples.

I thought about trying a different name (there is an identically named helper in Relocations.cpp that does something similar) but I couldn't come up with anything succinct. The best I could come up with was forEachInputSectionDescriptionUntilFalse().


================
Comment at: lld/ELF/OutputSections.cpp:478
+
+InputSection *getFirstInputSection(const OutputSection *os) {
+  InputSection *ret = nullptr;
----------------
Given that we only have 2 clients of forEachInputSectionDescription and no nested loops , it might be worth having two simpler functions that have their own loop
```
InputSection *getFirstInputSection(const OutputSection *os) {
  for (BaseCommand *base : os->sectionCommands)
    if (auto *isd = dyn_cast<InputSectionDescription>(base))
      if (!isd->sections.empty())
        return isd->sections.front();
  return nullptr;
}
```
getInputSections can remain the same as before.



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

https://reviews.llvm.org/D73047