[PATCH] D39406: Merge SymbolBody and Symbol into one class, SymbolBody.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Looks a good change to simplify the rest LLD code for me. Comments below:

1. nit: you want to fix description "r2681780" -> "r268178"
2. There are 3 very similar places that set or copy symbol's flags. I marked them as (1), (2), (3) below. It looks easy to forget to update these places when adding new flags, so I would suggest to introduce some helper, like `void lld::setSymbolAttributes(SymbolBody *S, uint8_t Binding, uint16_t VersionId ...);` or something different, perhaps some static member of SymbolBody, that would remove this code duplication and make code less error prone.



================
Comment at: lld/ELF/SymbolTable.cpp:262
     Sym->VersionId = Config->DefaultSymbolVersion;
     SymVector.push_back(Sym);
   } else {
----------------
(1)


================
Comment at: lld/ELF/Symbols.cpp:153
+  Traced = Sym.Traced;
+  InVersionScript = Sym.InVersionScript;
 }
----------------
(3)


================
Comment at: lld/ELF/Symbols.h:417
+  S->Traced = Sym.Traced;
+  S->InVersionScript = Sym.InVersionScript;
 
----------------
(2)


https://reviews.llvm.org/D39406