[PATCH] D22455: [ELF] Create output sections in LinkerScript class

[Rui Ueyama via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM with the following changes.


================
Comment at: ELF/LinkerScript.cpp:229
@@ +228,3 @@
+  for (SectionRule &R : Opt.Sections)
+    for (const std::unique_ptr<elf::ObjectFile<ELFT>> &F :
+         Symtab<ELFT>::X->getObjectFiles())
----------------
Do you need `elf::` specifier? If not, please remove.

================
Comment at: ELF/LinkerScript.cpp:238
@@ +237,3 @@
+  // Add all other input sections, which are not listed in script.
+  for (const std::unique_ptr<elf::ObjectFile<ELFT>> &F :
+       Symtab<ELFT>::X->getObjectFiles())
----------------
`elf::`

================
Comment at: ELF/LinkerScript.cpp:242
@@ +241,3 @@
+      if (!isDiscarded(S)) {
+        if(!S->OutSec)
+          AddInputSec(S, getOutputSectionName(S));
----------------
Add a space after `if`.

================
Comment at: ELF/LinkerScript.cpp:244-245
@@ +243,4 @@
+          AddInputSec(S, getOutputSectionName(S));
+      }
+      else
+        reportDiscarded(S, F);
----------------
`} else {` (or, in general, please run clang-format-diff.)

================
Comment at: ELF/Writer.cpp:222
@@ +221,3 @@
+
+  std::vector<std::unique_ptr<OutputSectionBase<ELFT>>> OwningSections =
+      ScriptConfig->DoLayout ? Script<ELFT>::X->createSections(Factory)
----------------
`OwningSections` was named such because Writer owned sections using this variable. It is no longer the case. So please rename something different, say just `Sections`.

================
Comment at: ELF/Writer.cpp:226-229
@@ +225,6 @@
+
+  std::for_each(OwningSections.begin(), OwningSections.end(),
+                [&](std::unique_ptr<OutputSectionBase<ELFT>> &Sec) {
+                  OutputSections.push_back(Sec.get());
+                });
+  finalizeSections();
----------------
Why don't you use a regular for-each loop?

  for (std::unique_ptr<OutputSectionBase<ELFT>> &S : OwningSections)
    OutputSections.push_back(S.get());


Repository:
  rL LLVM

https://reviews.llvm.org/D22455