[PATCH] D119294: [lld-macho] -flat_namespace for dylibs should make all externs interposable

[Jez Ng via Phabricator via llvm-commits]

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


================
Comment at: lld/MachO/SymbolTable.cpp:99
 
+  // With -flat_namespace, all extern symbols in dylibs are interposable.
+  bool interposable = config->namespaceKind == NamespaceKind::flat &&
----------------
modimo wrote:
> Could this be a `config` property rather than a per-symbol? It looks like a link invocation will always have flat_namespace or never have it.
ah this was done with an eye for future support of `-interposable_list`, which will allow specific symbols to be treated as interposable. I'll add a comment.


================
Comment at: lld/MachO/SyntheticSections.cpp:422
 
+static int16_t ordinalForSymbol(const Symbol &sym) {
+  if (const auto *dysym = dyn_cast<DylibSymbol>(&sym))
----------------
modimo wrote:
> >Note that the non-lazy binding and lazy binding sections now have to accommodate both DylibSymbols and (interposable) Defined symbols.
> 
> Aside from s/`DylibSymbol`/`Symbol`/g is the only other change to support this here?
> 
> Also, would recommended factoring out the s/`DylibSymbol`/`Symbol`/g change to a separate diff as a parent to make it easier to see what files have logic changes in them 
Pretty much yeah. I'll split out the NFC changes 👍


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119294