[PATCH] D15700: Split Undefined and UndefinedElf

[Rui Ueyama via llvm-commits]

ruiu added a subscriber: ruiu.
ruiu accepted this revision.
ruiu added a reviewer: ruiu.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits. Nice change.


================
Comment at: ELF/OutputSections.cpp:1335
@@ +1334,3 @@
+getElfSym(SymbolBody &Body) {
+  if (const auto *EBody = dyn_cast<Defined<ELFT>>(&Body))
+    return &EBody->Sym;
----------------
Why `const auto` instead of `auto`? (It's not wrong but looks like excessive.)

================
Comment at: ELF/SymbolTable.h:21
@@ -20,3 +20,3 @@
 template <class ELFT> class OutputSectionBase;
-template <class ELFT> class Undefined;
+class Undefined;
 
----------------
Sort.

================
Comment at: ELF/Symbols.cpp:82
@@ +81,3 @@
+                     bool CanKeepUndefined)
+    : Undefined(SymbolBody::UndefinedKind, N, IsWeak, Visibility, false) {
+  this->CanKeepUndefined = CanKeepUndefined;
----------------
Pass CanKeepUndefined instead of false.

================
Comment at: ELF/Symbols.cpp:124
@@ +123,3 @@
+namespace elf2 {
+template class UndefinedElf<ELF32LE>;
+template class UndefinedElf<ELF32BE>;
----------------
You can write

  template class lld::elf2::UndefinedElf<ELF...>;

================
Comment at: ELF/Symbols.h:133-136
@@ -132,6 @@
-//
-// FIXME: Another alternative is to give every symbol an Elf_Sym. To do that
-// we have to delay creating the symbol table until the output format is
-// known and some of its methods will be templated. We should experiment with
-// that once we have a bit more code.
-template <class ELFT> class ELFSymbolBody : public SymbolBody {
----------------
Thank you for removing this FIXME.


http://reviews.llvm.org/D15700