[PATCH] D17813: [ELF] - add support for relocations against local symbols when producing relocatable output.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/InputSection.cpp:139
@@ +138,3 @@
+      const Elf_Sym *Sym = this->File->getLocalSymbol(SymIndex);
+      auto I = Out<ELFT>::SymTab->Locals.find(Sym);
+      P->r_offset = RelocatedSection->getOffset(Rel.r_offset);
----------------
ruiu wrote:
> The value should always be available, right? So, you want to do
> 
>   uint32_t Idx = Out<ELFT>::SymTab->Locals[Sym];
> 
> instead of using find.
I dont like []. Problem is that [] operator creates and returns default value if there is no map entry. That can hide error, while accessing end iterator works like some kind of assert.
But here I think it is save to assume that value is really always available, so ok.

================
Comment at: ELF/OutputSections.h:246
@@ +245,3 @@
+  // Local symbol -> ID
+  std::map<const Elf_Sym *, unsigned> Locals;
+
----------------
ruiu wrote:
> Please use DenseMap. Also, I'd use uint32_t instead of unsigined.
Fixed, btw I just used the same type as assigned to entries: 
unsigned NumLocals = 0;
So I am not sure it is better, at least that was consistent.


http://reviews.llvm.org/D17813