[PATCH] D56089: [ELF] A shared object is needed if any of its occurrences is needed

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D56089#1341384 <https://reviews.llvm.org/D56089#1341384>, @ruiu wrote:

> Quadratic behavior should be fine as a square of a few thousand is still small. We don't need to worry too much about it. That said, if you don't like it, it is also fine as long as the code is as easy to understand as the more dumb-but-simple code. I'd think that the current comment is not suffice because it only explains what that code does. Please copy my comment which explains why we want it.
>
> Also please avoid try_emplace as the code looks a bit too smart. The actual values of R.first->second and R->second are not obvious. I'd write something like this.
>
>   F->parseSoName();
>   if (errorCount())
>     return;
>   
>   // DSO are uniquified by soname.
>   InputFile *&Existing = SoNames[F->SoName];
>   if (Existing) {
>     if (F->IsNeeded)
>       cast<SharedFile<ELFT>>(Existing)->IsNeeded = true;
>     return;
>   }
>   Existing = F;


`DenseMap::try_emplace` is a C++17 flavored function name that powers `insert`, which has been used in 30+ llvm files  and 20+ clang files. I know lld uses a slightly different style, so I have spelled out the two components. `WasInserted` is picked according to the existing name convention in another place of lld.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56089