[PATCH] D35873: Merge OutputSectionCommand and OutputSection

[Rui Ueyama via Phabricator via llvm-commits]

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

LGTM

It looks like this is the right thing to do. Please rename Cmd Sec if a variable is of OutputSection.



================
Comment at: ELF/LinkerScript.cpp:609
+                            [&](BaseCommand *Base) {
+                              if (auto *Cmd = dyn_cast<OutputSection>(Base))
+                                return !Cmd->Live;
----------------
Cmd -> Sec


================
Comment at: ELF/LinkerScript.cpp:631
   for (int I = 0, E = Opt.Commands.size(); I != E; ++I) {
-    auto *Cmd = dyn_cast<OutputSectionCommand>(Opt.Commands[I]);
+    auto *Cmd = dyn_cast<OutputSection>(Opt.Commands[I]);
     if (!Cmd)
----------------
Cmd -> Sec


================
Comment at: ELF/LinkerScript.cpp:651
   for (BaseCommand *Base : Opt.Commands) {
-    if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base)) {
+    if (auto *Cmd = dyn_cast<OutputSection>(Base)) {
       Cmd->MemRegion = findMemoryRegion(Cmd);
----------------
Cmd -> Sec


================
Comment at: ELF/MapFile.cpp:103
 template <class ELFT>
-void elf::writeMapFile(llvm::ArrayRef<OutputSectionCommand *> Script) {
+void elf::writeMapFile(llvm::ArrayRef<OutputSection *> Script) {
   if (Config->MapFile.empty())
----------------
Rename Script.


================
Comment at: ELF/MapFile.h:18
+class OutputSection;
+template <class ELFT> void writeMapFile(llvm::ArrayRef<OutputSection *> Script);
 } // namespace elf
----------------
Remove `Script`.


================
Comment at: ELF/OutputSections.h:42
 // non-overlapping file offsets and VAs.
-class OutputSection final : public SectionBase {
+class OutputSection final : public BaseCommand, public SectionBase {
 public:
----------------
I really don't want to use multiple inheritance, but BaseCommand is so small that it is probably okay.


================
Comment at: ELF/OutputSections.h:93
+
+  MemoryRegion *MemRegion = nullptr;
+  Expr AddrExpr;
----------------
Add a comment saying that the members below are for linker scripts.


================
Comment at: ELF/Relocations.cpp:1009
 
-static uint32_t findEndOfFirstNonExec(OutputSectionCommand &Cmd) {
+static uint32_t findEndOfFirstNonExec(OutputSection &Cmd) {
   for (BaseCommand *Base : Cmd.Commands)
----------------
Cmd -> Sec


================
Comment at: ELF/Writer.cpp:486
   for (BaseCommand *Base : Script->Opt.Commands) {
-    auto *Cmd = dyn_cast<OutputSectionCommand>(Base);
+    auto *Cmd = dyn_cast<OutputSection>(Base);
     if (!Cmd)
----------------
Cmd -> Sec


================
Comment at: ELF/Writer.cpp:1182
     for (BaseCommand *Base : Script->Opt.Commands)
-      if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base))
-        if (Cmd->Sec)
-          addStartStopSymbols(Cmd->Sec);
+      if (auto *Cmd = dyn_cast<OutputSection>(Base))
+        addStartStopSymbols(Cmd);
----------------
Sec


https://reviews.llvm.org/D35873