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

[Rui Ueyama via llvm-commits]

ruiu 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)
----------------
Do you still need this function?

================
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)
----------------
`OutSec` as a name for a command is very confusing.

================
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())
----------------
Flip `if` conditions and do continue to reduce indentation levels.

================
Comment at: ELF/LinkerScript.h:45
@@ +44,3 @@
+  OutputSectionKind,
+  InputSectionKind
+};
----------------
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.

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


https://reviews.llvm.org/D22617