[PATCH] D27040: [ELF] - Add support for access to most of synthetic sections from linkerscript.
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 24 02:29:47 PST 2016
grimar added inline comments.
================
Comment at: ELF/SyntheticSections.h:35
virtual void finalize() {}
+ virtual bool isPresent() const { return true; }
----------------
ruiu wrote:
> `isPresent` sound a bit odd to me, because as soon as you instantiate objects, they are present. How about `empty`?
Done.
================
Comment at: ELF/Writer.cpp:230
+ Symtab<ELFT>::X->Sections.push_back(Targ);
+};
+
----------------
ruiu wrote:
> 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.
I did that though I find this a bit bulky code. I think we may want to refactor it in this way separatelly.
================
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
----------------
ruiu wrote:
> `removeUnusedSyntheticSections` because we have other synthetic things such as synthetic symbols.
Done.
================
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);
----------------
ruiu wrote:
> Use `llvm::reverse`.
Done.
https://reviews.llvm.org/D27040
More information about the llvm-commits
mailing list