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

[Jez Ng via Phabricator via llvm-commits]

int3 marked an inline comment as done.
int3 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:
> 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79460