[PATCH] D61895: Introduce CommonSymbol.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu marked an inline comment as done.
ruiu added a comment.

Looks like it is not easy to make changes to a patch in a patch series as it causes merge conflicts. It's still better than merging them into one big patch and send it for review, but splitting a patch makes my life a bit harder when it comes to updating it. Do you have any critical comments that need to be addressed? I'll make changes as you guys requested before committing, so if this patch looks generally OK, could you stamp on it for me?



================
Comment at: lld/ELF/SymbolTable.cpp:496
     for (Symbol *Sym : SymVector) {
       if (!Sym->isDefined())
         continue;
----------------
MaskRay wrote:
> `getDemangledSyms` is transitively called by `scanVersioScript` before `replaceCommonSymbols`. Is a `isCommon()` check needed here?
Yes, I guess so. I'll do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61895