[PATCH] D29330: Replace MergeOutputSection with synthetic input section MergeSection.

[Rafael Avila de Espindola via llvm-commits]

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar accepted this revision.
> grimar added a comment.
> This revision is now accepted and ready to land.
>
> LGTM with single comment, though is up to you and Rui I think to select what is looking better there.
>
>
>
> ================
> Comment at: ELF/InputSection.cpp:133
> +template <class ELFT>
> +OutputSectionBase *InputSectionBase<ELFT>::getOutputSection() const {
> +  if (auto *MS = dyn_cast<MergeInputSection<ELFT>>(this))
> ----------------
> By the way, diff5 of my patch solved this in a bit different way:
>
> ```
> template <class ELFT>
> void OutputSection<ELFT>::addSection(InputSectionData *C) {
> ...
>   if (auto *Sec = dyn_cast<MergeSyntheticSection<ELFT>>(C))
>     Sec->setOutputSection(this);
> ```
>
> setOutputSection just assigned OutSec member. I see 2 positive things of doing that:
> 1) We use public member OutSec in many places I think, having another getOutputSection() is a bit confuzing
> 2) It just simplifies things.
>
> Rui did not like that piece of code it seems, though probably it was better than one more method,

I prefer the method. Ideally we should refactor the class hierarchy so
that each section just has one pointer to its "container", be it a
synthetic merge section or an output section.

Cheers,
Rafael