[PATCH] D123433: [lld-macho][nfc] Use includeInSymtab for all symtab-skipping logic

[Jez Ng via Phabricator via llvm-commits]

int3 marked 2 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/ConcatOutputSection.cpp:332
+            thunkName, /*file=*/nullptr, thunkInfo.isec, /*value=*/0, thunkSize,
+            /*isWeakDef=*/false, /*isPrivateExtern=*/true,
             /*isThumb=*/false, /*isReferencedDynamically=*/false,
----------------
thakis wrote:
> For the lines that are just reformatted without changes, maybe commit those in a separate commit.
oh yeah I had initially added the extra param to `addDefined` as well but removed it after, hence the formatting changes. will revert those


================
Comment at: lld/MachO/SymbolTable.cpp:233
                                    bool referencedDynamically) {
-  assert(!isec || !isec->getFile()); // See makeSyntheticInputSection().
   Defined *s =
----------------
thakis wrote:
> Doesn't this still make sense to explain the `/*file=*/nullptr` (instead of the `isec->getFile()` you'd expect -- or at least I expected that.)
sure, I can leave it in. IMO it's conceivable that synthetic symbols could have an associated InputSection, but it's true that we don't create any of those at the moment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123433