[PATCH] D116913: [lld-macho] Add --start-lib --end-lib

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!



================
Comment at: lld/MachO/InputFiles.cpp:1619
+  symbols.resize(obj->symbols().size());
+  for (auto it : llvm::enumerate(obj->symbols())) {
+    const lto::InputFile::Symbol &objSym = it.value();
----------------
MaskRay wrote:
> MaskRay wrote:
> > oontvoo wrote:
> > > int3 wrote:
> > > > no need for `llvm::`
> > > Sorry, I'm missing something here but why not simply ` for (const lto::InputFile::Symbol &objSym : obj->symbols())` ?
> > > IOWs, why is `llvm::enumerate` needed?
> > `llvm::foo` for STL-like helpers (lower_bound, find_if, etc) has an advantage: prevent the confusing argument-dependent lookup. In other components of llvm-project `llvm::foo` for STL-like helpers is generally recommended.
> > 
> > So I am going to defend for the usage a bit...
> `symbols[it.index()] ` is needed by `-ObjC` for ObjC symbol checking. It has another advantage: when a symbol table refactoring is done in the future, we can transfer the symbol table to the non-lazy state. See D116390 for ELF.
mm yeah, I have also been including `llvm::` in front of LLVM functions which have STL counterparts of the same name. My reasoning was that since `std::enumerate` doesn't exist, `llvm::` wasn't needed here... but I can also see the argument for using `llvm::` for all STL-like things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116913