[PATCH] D22617: [ELF] - Linkerscript: add InputSectionDescription command to LS parser.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 21 05:16:49 PDT 2016


grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:202
@@ -197,2 +201,3 @@
 template <class ELFT>
 StringRef LinkerScript<ELFT>::getOutputSection(InputSectionBase<ELFT> *S) {
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
----------------
ruiu wrote:
> Do you still need this function?
getOutputSection() was used in LinkerScript<ELFT>::isDiscarded().
I changed logic to avoid the using as "/DISCARD/" can just be processed as
special section kind.

================
Comment at: ELF/LinkerScript.cpp:247
@@ +246,3 @@
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
+    if (auto *OutSec = dyn_cast<OutputSectionCommand>(Base.get()))
+      for (const std::unique_ptr<BaseCommand> &Cmd : OutSec->Commands)
----------------
ruiu wrote:
> `OutSec` as a name for a command is very confusing.
Changed to OutSec.

================
Comment at: ELF/LinkerScript.cpp:249
@@ +248,3 @@
+      for (const std::unique_ptr<BaseCommand> &Cmd : OutSec->Commands)
+        if (auto *InSec = dyn_cast<InputSectionDescription>(Cmd.get()))
+          for (ObjectFile &F : Symtab<ELFT>::X->getObjectFiles())
----------------
ruiu wrote:
> Flip `if` conditions and do continue to reduce indentation levels.
Done.

================
Comment at: ELF/LinkerScript.h:45
@@ +44,3 @@
+  OutputSectionKind,
+  InputSectionKind
+};
----------------
ruiu wrote:
> grimar wrote:
> > evgeny777 wrote:
> > > See comment above:
> > > 
> > > > This enum represents what we can observe in SECTIONS tag of script.
> > > > (https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS)
> > > 
> > > If you follow the link:
> > > 
> > > ```
> > > Each sections-command may of be one of the following:
> > > 
> > > - an ENTRY command (see Entry command)
> > > - a symbol assignment (see Assignments)
> > > - an output section description
> > > - an overlay description
> > > ```
> > > 
> > > 
> > > 
> > > 
> > Well, comment is depricated. I guess we do not want to treat SectionsCommandKind as written anymore, so it can be removed completely,
> > Rui, what do you think ?
> We should just remove this comment and leave the link to the documentation.
Done.

================
Comment at: ELF/LinkerScript.h:76
@@ +75,3 @@
+  std::vector<StringRef> Patterns;
+  bool Keep = false;
+};
----------------
ruiu wrote:
> Now you can remove this.
Done.


https://reviews.llvm.org/D22617





More information about the llvm-commits mailing list