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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 08:42:07 PST 2017


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,


https://reviews.llvm.org/D29330





More information about the llvm-commits mailing list