[PATCH] D33024: [Support] Move LLD parallel algorithms to LLVM.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/COFF/Writer.cpp:748
       memset(SecBuf, 0xCC, Sec->getRawSize());
-    for_each(parallel::par, Sec->getChunks().begin(), Sec->getChunks().end(),
-             [&](Chunk *C) { C->writeTo(SecBuf); });
+    for_each(llvm::parallel::par, Sec->getChunks().begin(),
+             Sec->getChunks().end(), [&](Chunk *C) { C->writeTo(SecBuf); });
----------------
chandlerc wrote:
> zturner wrote:
> > chandlerc wrote:
> > > I find the fact that `for_each` is now getting looked up via ADL on the `llvm::parallel::par` type ... disturbing.
> > I can certainly qualify the function call, but I don't know of a way to disable ADL if that's what you're suggesting.
> It's "easy": `foo(a, b, c)` uses ADL and `(foo)(a, b, c)` doesn't use ADL. ;]
> 
> I'm not suggesting using `(for_each)` though. That seems a bit crazy.
> 
> Writing `llvm::foo` will also disable ADL. ADL only fires for unqualified names. But then writing the code gets super ugly:
> 
>   llvm::for_each(llvm::parallel::par, ...);
> 
> Yuck. I guess I mildly prefer that? The standard sure chose an API that ends up requiring a lot of characters to type.
> 
> 
> What do you or Rui think? I don't have strong feelings about which way you go here, so would generally defer to your or Rui's judgement.
It's worse than that, because it's going to be `llvm::parallel::for_each(llvm::parallel::par, ...)`.  We could nuke the intermediate `parallel` namespace and go with `llvm::for_each(llvm::par, ...)`, or we could do `using llvm::parallel` at the top of a file.  The namespace is sufficiently scoped that bringing it in shouldn't be too onerous.

Personally I'm fine with any approach, including the ADL-based approach.  That said, I don't think there's anything we can do to the API itself to alleviate this, so perhaps we don't need to do anything.  Just leave it up to the users.


https://reviews.llvm.org/D33024