[PATCH] D83603: [lld-macho] Support __dso_handle for C++

Wed Jul 15 12:28:31 PDT 2020

int3 marked an inline comment as done.
int3 added inline comments.

================
Comment at: lld/MachO/Symbols.h:139
+
+  static const constexpr StringRef name = "___dso_handle";
+
----------------
int3 wrote:
> int3 wrote:
> > pcc wrote:
> > > compnerd wrote:
> > > > What do you think of making this ever so slightly more expensive?  I think that we should actually make this computed - the name of the symbol is `__dso_handle` which is decorated with the user label prefix of `_` to give you `___dso_handle`.  We have the information about the user label prefixes in LLVM, why not make that explicit here?  I don't think that the cost is really that high.
> > > FWIW, in general I'm not a fan of the idea of computing string constants like this, as it makes it harder to grep the code for strings like `___dso_handle`.
> > I wasn't aware that LLVM had info about user label prefixes (though that makes sense). Happy to change. I suppose I'll want something like `getTargetInfo().getDataLayout().getGlobalPrefix()`
> hmm `DataLayout` is under the `IR` folder though. I'm not sure how else to get at the user label, but we could probably just make it a global constant in lld-macho since the prefix applies to all macOS targets?
saw @pcc's comment later on, and I think it's a good reason to keep `__dso_handle`... also it's nice to be able to write the string inline in the class declaration rather than in the .cpp file.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83603