[PATCH] D12545: [elf2] Add basic archive file support.

[Rafael Espíndola via llvm-commits]

+  auto ItOrErr = Sym->getMember();

Non obvious type. Please don't use auto.

Can you extend the test to cover us not adding a member twice? Should
be as simple as adding a second undefined symbol defined by the same
member.

+  llvm::DenseMap<uint64_t, bool> Seen;

Can this be a DenseSet?

+// Returns a buffer pointing to a member file containing a given symbol.
+MemoryBufferRef ArchiveFile::getMember(const Archive::Symbol *Sym) {

The comment is effectively duplicated from the header.



On 3 September 2015 at 17:57, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> +    case SymbolBody::LazyKind:
> +      assert("Lazy symbols shouldn't get here.");
>
> You want an llvm_unreachable, no? That assert never fails.
>
> On 3 September 2015 at 17:48, Michael Spencer <bigcheesegs at gmail.com> wrote:
>> Bigcheese updated this revision to Diff 33976.
>> Bigcheese added a comment.
>>
>> Address comments.
>>
>>
>> http://reviews.llvm.org/D12545
>>
>> Files:
>>   ELF/Driver.cpp
>>   ELF/InputFiles.cpp
>>   ELF/InputFiles.h
>>   ELF/SymbolTable.cpp
>>   ELF/SymbolTable.h
>>   ELF/Symbols.cpp
>>   ELF/Symbols.h
>>   ELF/Writer.cpp
>>   test/elf2/archive.s
>>