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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: lld/MachO/Config.h:41
 
+constexpr uint32_t encodeVersion(const llvm::VersionTuple &version) {
+  return ((version.getMajor() << 020) |
----------------
int3 wrote:
> why `constexpr`?
cause it implies inline, but given that VersionTuple doesn't have a constexpr ctor yet, perhaps, we can replace it with inline.


================
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;
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103505