[PATCH] D29021: [ELF] - Stop handling local symbols in a special way.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D29021#653797, @ruiu wrote:

> Or, if you find that adding a new field to Symbol class doesn't really hurt any metrics, you can simply add it. Or, even if it slows down the linker a little bit, you may still be able to convince that that is necessary. I believe adding a new field is the simplest and sufficient way to do what you want to do, but I think you want to stop to think and investigate a few different ways, and then make your conclusion (because all I said were my guesses and not conclusions in any sense.)


Mmmm. Sorry, did you get this had LGTM and was already landed ? (I am not sure, I thought we discussing post commit details.)

I think I agree with Rafael that for now we can go with this solution and see if we want to optimize for -r or not separatelly later.
-relocatable output is not general use case and I think I am agree extending Symbol simply not worth doing if can be avoided.
No matter if that slows down linker or not.

This patch removed KeptLocalSyms and localized symbols handling in one place, what IMO made code cleaner. So it was useful to have.
And even if we decide to add field to Symbol, this patch helps to do that in a easy way still.


Repository:
  rL LLVM

https://reviews.llvm.org/D29021