[PATCH] D19752: ELF: New symbol table design.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: ELF/InputFiles.h:190
@@ -185,5 +189,3 @@
 
-  void parse();
-
-  llvm::MutableArrayRef<LazyObject> getLazySymbols() { return LazySymbols; }
+  void parse(SymbolTableBase *Symtab);
 
----------------
ruiu wrote:
> Can we merge SymbolTableBase with SymbolTable<ELFT> if you template this, ArchiveFile::parse and BitcodeFile::parse functions?
I can, done. I'm not sure if it's a good idea to add more template bloat like this, but we can probably think about that separately.

================
Comment at: ELF/Symbols.h:427
@@ +426,3 @@
+      DefinedSynthetic<llvm::object::ELF64LE>, Undefined,
+      SharedSymbol<llvm::object::ELF64LE>, LazyArchive, LazyObject>
+      Body;
----------------
rafael wrote:
> Using ELF64LE is here is problematic as the SharedSymbol has a pointer to Elf_Sym. As silly as it  sounds I think you need to include the various ELFTs in the "union".
> 
> And can this be just a plain union?
All I need is an appropriately sized and aligned char array. That's what `AlignedCharArrayUnion` provides. I never actually access this field as a `SharedSymbol<ELF64LE>` for example (unless of course I'm in a templated function with `ELFT=ELF64LE`). I'm making an assumption here that ELF32* data structures are not larger or more aligned than ELF64* data structures (and the same for LE and BE), but if they somehow are not, that's what the `static_assert`s below are for.

I think this could be a union, but we'd need default constructors in every derived class of `SymbolBody`, and I think we'd need to use the unrestricted unions feature of C++, which MSVC doesn't support until 2015.


http://reviews.llvm.org/D19752