[PATCH] D77893: [lld] Merge Mach-O input sections

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: lld/MachO/MergedOutputSection.h:19
+
+class MergedOutputSection : public OutputSection {
+public:
----------------
Ktwu wrote:
> Ktwu wrote:
> > int3 wrote:
> > > smeenai wrote:
> > > > Could you add a class comment about what this represents?
> > > > 
> > > > I'd prefer renaming `OutputSection` to `OutputSectionBase` and `MergedOutputSection` to just `OutputSection`, to be more in line with ELF, but I don't feel super strongly about that.
> > > +1 on the renaming
> > Sure.
> Er, actually, I like MergedOutputSection because it conveys more information about what kind of OutputSection it is. Given how many of the other classes used don't use -Base in their name -- InputSections, OutputSegments -- it feels clunky to have an OutputSectionBase.
> 
> @int3 do you feel strongly about renaming this? 
Sure, I'm good with the parent class being named `OutputSection` and having specific subclasses representing the different types of output sections. I'm not the biggest fan of the `MergedOutputSection` name because to me it suggests output sections being merged together rather than input sections being merged into a single output section, but I can't think of anything better either, so I'm good with it if we can't come up with a better name.

Naming is hard :D


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77893/new/

https://reviews.llvm.org/D77893