[PATCH] D27040: [ELF] - Add support for access to most of synthetic sections from linkerscript.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/SyntheticSections.h:35
   virtual void finalize() {}
+  virtual bool isPresent() const { return true; }
 
----------------
`isPresent` sound a bit odd to me, because as soon as you instantiate objects, they are present. How about `empty`?


================
Comment at: ELF/Writer.cpp:230
+  Symtab<ELFT>::X->Sections.push_back(Targ);
+};
+
----------------
grimar wrote:
> evgeny777 wrote:
> > This looks better
> > 
> > ```
> > auto add = [](InputSection<ELFT> *IS) {
> >   Symtab<ELFT>::X->Sections.push_back(IS);
> >   return IS;
> > };
> > ```
> > 
> Unfortunately this will not work. You can not assign base class pointer to derived.
> I applied the idea of your suggestion though.
It seems unnecessarily complicated. I'd just remove this function and inline it.


================
Comment at: ELF/Writer.cpp:255
+  if (needsInterpSection<ELFT>())
+    doAdd<ELFT>(In<ELFT>::Interp, createInterpSection<ELFT>());
+  else
----------------
evgeny777 wrote:
> ```
> In<ELFT>::Interp = add(createInterpSection<ELFT>());
> ```
For example, I'd write

  In<ELFT>::Interp = createInterpSection<EFLT>();
  Symtab<ELFT>::X->Sections.push_back(In<ELFT>::Interp);


================
Comment at: ELF/Writer.cpp:878
+template <class ELFT>
+static void removeUnusedSynthetics(std::vector<OutputSectionBase *> &V) {
+  // Synthetic sections are placed after all regular ones. We iterate over them
----------------
`removeUnusedSyntheticSections` because we have other synthetic things such as synthetic symbols.


================
Comment at: ELF/Writer.cpp:881-882
+  // all and exit at first non-synthetic.
+  for (auto I = Symtab<ELFT>::X->Sections.rbegin();
+       I != Symtab<ELFT>::X->Sections.rend(); ++I) {
+    SyntheticSection<ELFT> *SS = dyn_cast<SyntheticSection<ELFT>>(*I);
----------------
Use `llvm::reverse`.


https://reviews.llvm.org/D27040