[PATCH] D27415: [ELF] - Replace MergeOutputSection with synthetic input section MergeSection.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputSection.cpp:134
+  case Merge: {
+    const MergeInputSection<ELFT> *MS = cast<MergeInputSection<ELFT>>(this);
+    if (!MS->Live)
----------------
ruiu wrote:
> Use auto.
Done.


================
Comment at: ELF/OutputSections.cpp:133
+
+  auto *SS = dyn_cast<SyntheticSection<ELFT>>(C);
+  if (SS && SS->Mergeable)
----------------
ruiu wrote:
> Can you keep the scope as narrow as possible? `SS` has unnecessarily large scope here. If you do this, it'll be much narrower.
> 
>   if (auto *Sec = dyn_cast<...>())
>     if (Sec->Mergeable)
>       ...;
Ok, done.


================
Comment at: ELF/OutputSections.cpp:135
+  if (SS && SS->Mergeable)
+    static_cast<MergeSection<ELFT> *>(SS)->setOutputSection(this);
+
----------------
ruiu wrote:
> You seems to be reinventing dynamic class dispatching. Please define dyn_cast<> for MergeSection<ELFT>.
Done.


https://reviews.llvm.org/D27415