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

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 8 23:04:11 PDT 2020


int3 added inline comments.


================
Comment at: lld/MachO/Writer.cpp:117
 
-    if (!seg->isNeeded())
+    if (seg->getSections().empty())
       return;
----------------
int3 wrote:
> Ktwu wrote:
> > My bad, was this an actual bug? Why not hide the empty() check within a function?
> It wasn't an actual bug, but the check here was to avoid calling `firstSection()` below on an empty sections vector. I think using `isNeeded()` which also checks if any contained sections are needed obscures this (plus it confused me for a bit as to why we had to check isNeeded() here when we were already checking it before calling `writeTo()`).
> 
> I'll add a comment here to make its purpose clear
Actually, I think it was an actual bug: the whole purpose of this check is for `__LINKEDIT`, which is the only segment that may have `writeTo()` called on it even if it contained no sections. But `isNeeded()` will return `true` for `__LINKEDIT`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79460





More information about the llvm-commits mailing list