[PATCH] D15700: Split Undefined and UndefinedElf

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 17:07:10 PST 2015

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;

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.


More information about the llvm-commits mailing list