[PATCH] D37489: Linker script handing of file patterns with COMMON defs

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/Symbols.h:177
   uint64_t Size;
+  BssSection *Section;
 };
----------------
  Section = nullptr


================
Comment at: lld/ELF/SyntheticSections.cpp:57-58
 
-template <class ELFT> static std::vector<DefinedCommon *> getCommonSymbols() {
-  std::vector<DefinedCommon *> V;
-  for (Symbol *S : Symtab->getSymbols())
-    if (auto *B = dyn_cast<DefinedCommon>(S->body()))
-      V.push_back(B);
-  return V;
-}
-
-// Find all common symbols and allocate space for them.
-template <class ELFT> InputSection *elf::createCommonSection() {
+std::vector<InputSection *> elf::createCommonSections()
+{
+  std::vector<InputSection *> SV;
----------------
clang-format


================
Comment at: lld/ELF/SyntheticSections.cpp:59
+{
+  std::vector<InputSection *> SV;
   if (!Config->DefineCommon)
----------------
By convention, `Ret` is preferred over `SV`. In general, I found that giving the same name to a variable regardless of its type is more readable than naming it based on type. So, I prefer naming a symbol (whether it is defined or not) `Sym` instead of `DC` or something like that.


================
Comment at: lld/ELF/SyntheticSections.cpp:64
+  for (Symbol *S : Symtab->getSymbols()) {
+    auto *DC = dyn_cast<DefinedCommon>(S->body());
+    if (!DC || !DC->Live)
----------------
So, let's name this Sym


================
Comment at: lld/ELF/SyntheticSections.h:757
   static BuildIdSection *BuildId;
-  static InputSection *Common;
+  static std::vector<InputSection *> Commons;
   static SyntheticSection *Dynamic;
----------------
I think you can now remove this member.


https://reviews.llvm.org/D37489