[PATCH] D67301: [LLD] Use the unified llvm demangle frontend function. NFC.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 24 22:35:39 PDT 2019


MaskRay added a comment.

In D67301#1680905 <https://reviews.llvm.org/D67301#1680905>, @rnk wrote:

> In D67301#1665612 <https://reviews.llvm.org/D67301#1665612>, @MaskRay wrote:
>
> > We haven't even heard people complaining about lack of demangling of `__Z`, `___Z` or `___Z` prefixed names. I still lean towards keeping the `"_Z"` check for ELF to avoid false positive.
>
>
> Weighing this desire to not change functionality against the cost of adding an extra boolean default argument to control the behavior, I'd prefer to change behavior to be consistent everywhere.
>
> I guess we can't predict which bug is more likely: failing to demangle something the user wanted us to demangle, or unintentionally demangling a real C symbol starting with `Z`. Regardless of which bug appears down the road, I favor the solution that takes less code and makes LLVM tools behave consistently.


Is there a scenario where both Itanium and Microsoft mangling schemes should be tried? (mingw?) If no, separate entry points for itaniumDemangle and microsoftDemangle may be better. I did not expect the solution to end up with an extra parameter `StrictPrefix`. The additional complexity was not what I intended. If mingw has to live with both mangling schemes, I want to first check if the demangling code will be used in more than one places. If yes, I am not against deleting StrictPrefix. The overhead to demangle `__Z`, `___Z` or `___Z` prefixed names for ELF should be small.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67301/new/

https://reviews.llvm.org/D67301





More information about the llvm-commits mailing list