[PATCH] D42960: [ELF] Add .eh_frame pieces to map file

Rafael Ávila de Espíndola via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 8 05:55:59 PST 2018


rafael added a comment.

Rui Ueyama via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> ruiu added inline comments.
> 
> ================
>  Comment at: ELF/InputSection.h:347
> 
> -std::string toString(const elf::InputSectionBase *);
>  +std::string toString(const elf::InputSectionBase *, uint64_t SecOffset = 0);
> 
>   } // namespace lld
> 
>  ----------------
> 
> I don't think this is a good idea. lld::toString(T) is an overloaded function dispatched by its parameter type. But you are now defining a different function which you only use in MapFile.cpp. I'd move this back to the original file.

My objection to the original implementation was that it was looking at
the output of toString, effectively knowing exactly what toString does
and breaking the modularity.

Would you be OK with having

std::string lld::someName(const InputSectionBase *Sec, uint64_t Offset);

and implementing std::string lld::toString(const InputSectionBase *Sec)
as someName(Sec, 0)?

Cheers,
Rafael


https://reviews.llvm.org/D42960





More information about the llvm-commits mailing list