[PATCH] D103505: [lld][MachO] Add first bits to support special symbols

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:978
+  std::tie(endVersion, symbolName) = name.split('$');
+  symbolName = symbolName.drop_back(1 /* $ */);
+  // TODO: ld64 contains some logic for non-empty symbolName as well.
----------------
I'm guessing the `$` is there as an extension point so that additional stuff can be added later if necessary. So maybe

    std::tie(symbolName, rest) = name.split('$');

might be better, in case more stuff is in fact added later?


================
Comment at: lld/MachO/InputFiles.cpp:963-964
+  std::tie(action, name) = name.drop_front(4 /* $ld$ */).split('$');
+  if (action.empty() || action != "previous")
+    return false;
+
----------------
alexshap wrote:
> int3 wrote:
> > Perhaps we should return `true` here, since these are clearly not meant to be interpreted as regular symbols. (Same for the other `return false` lines below.)
> > 
> > Also maybe leave a comment / FIXME about the other actions?
> I was thinking about it too, at the moment it's like "we return true only if we have successfully processed this symbol and do not change the behavior otherwise",  this was also kind of affected by what Nico's patch was doing, but yeah, perhaps, it would be better to return true in all these cases.
(My patch hadn't a lot of thought put into it, it's more a sketch than a patch I agree skipping everything starting with `$ld` makes sense; maybe we should warn on the ones we don't understand.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103505