[PATCH] D79460: [lld-macho] Follow-up to D77893

[Kellie Medlin via Phabricator via llvm-commits]

Ktwu added inline comments.


================
Comment at: lld/MachO/OutputSegment.cpp:43
     OutputSection *os = i.second;
-    count += (os->isHidden() ? 0 : 1);
+    count += (os->isNeeded() && !os->isHidden() ? 1 : 0);
   }
----------------
int3 wrote:
> int3 wrote:
> > Ktwu wrote:
> > > I personally find this more complicated to read :(
> > > 
> > > Knowing that hidden state isn't as simple as checking isHidden() -- isNeeded() is also needed -- seemed confusing to me :/
> > I agree that sprinkling `isNeeded()` checks around isn't pretty, but the flip side is that there's no longer an implicit requirement that every implementation of `OutputSection` must maintain the invariant that `isNeeded() == false => isHidden() == false`.
> > 
> > But see also https://reviews.llvm.org/D77893#2022314 -- I think we could further clean things up by filtering out all the unneeded sections earlier.
> Actually, I can do the filtering in this diff, inside `sortOutputSegmentsAndSections`, instead of depending on the larger proposed refactor. How does that sound?
Yup, do it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79460