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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D67301#1681906 <https://reviews.llvm.org/D67301#1681906>, @MaskRay wrote:

> 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?)


No, practically, only one of them should be possible at a given time. But I don't see why that is relevant to the discussion at hand.

> If no, separate entry points for itaniumDemangle and microsoftDemangle may be better.

The distinction between itanium and microsoft isn't an issue here, at all.

They have strictly very different mangling; microsoft mangled names start with a `?` while itanium mangled ones start with `_Z`. There is no risk of mixing them up at all.

On i386, all symbols (except for microsoft mangled and certain other calling conventions) start with an extra underscore, so itanium mangled names start with `__Z`. This is currently handled by always manually stripping one underscore on i386 before calling the demangler, but could be skipped as well if we use a more tolerant demangler. It could also be kept to avoid the risk of demangling a symbol like `Znwy` that originally didn't start with an underscore.

> 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.

It's not at all about the two different mangling schemes.

It's only about whether we artifically should make the general itanium demangler routine more strict than it is right now, or accept that there's a small risk of demangling something that isn't supposed to, but this is already a risk that all existing callers of the general llvm demangle library are ok with.


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

https://reviews.llvm.org/D67301