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

[Kellie Medlin via Phabricator via llvm-commits]

Ktwu marked 6 inline comments as done and an inline comment as not done.
Ktwu added inline comments.


================
Comment at: lld/MachO/MergedOutputSection.h:19
+
+class MergedOutputSection : public OutputSection {
+public:
----------------
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? 


================
Comment at: lld/test/MachO/section-merge.s:45
+# DATA:           SectionData (
+# DATA-NEXT:        0000: 48C7C001 000000C3 48C7C000 000000C3
+# DATA-NEXT:      )
----------------
smeenai wrote:
> Might be easier to make sense of this as assembly (`llvm-objdump -d`)
Is this a big deal? I compared this output to ld; I don't care about the content so much as making sure it matches what ld outputs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77893