[PATCH] D48298: [ELF] Uniquify --wrap list.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 19 01:32:52 PDT 2018


grimar added inline comments.


================
Comment at: lld/trunk/ELF/Driver.cpp:1310
+  for (StringRef Name : Wraps)
+    Symtab->addSymbolWrap<ELFT>(Name);
 
----------------
Honestly saying, it looks weird.

Looking at the code I would say we are trying to wrap the symbols in the particular
alphabetical order, though that is not true. Calling `sort` it not consistent with how we
handle the command line options in general I believe.

Instead of this, I would suggest to add a small piece into `addSymbolWrap`:

```
template <class ELFT> void SymbolTable::addSymbolWrap(StringRef Name) {
....
  // Do not wrap the same symbol twice.
  if (llvm::find_if(WrappedSymbols, [&](const WrappedSymbol &S) {
        return S.Sym == Sym;
      }) != WrappedSymbols.end())
    return;
```

Rui, what do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D48298





More information about the llvm-commits mailing list