[PATCH] D67301: [LLD] Unify the demangleItanium and demangleMSVC functions. NFC.

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

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

> > Should the _ prefix (for itanium symbols in i386 mode) be stripped by the COFF wrapper function, or by the common demangle function (matching _Z or __Z)?
>
> Maybe, I forget if the extra _ comes before the __imp_ prefix or after it when dllimported.


It's after the `__imp_` prefix, so the demangler could handle it.

> But there's another consideration, which is that this code could hypothetically be shared with a MachO linker implementation, where the extra underscore prefix is also present. IMO it's best for any generic demangler to match any number of underscores followed by Z.

I'm not entirely decided on whether I think this is a good thing or not.

The caller knows with certainty whether there should be an extra underscore prefix or not, as it is well defined (always in MachO, never in ELF, plus on i386 in COFF). But if we make the demangler implicitly handle both cases, it can also accidentally e.g. try to demangle any symbol which just starts with a capital Z, without any real underscore prefix, on i386 in COFF or in MachO. (I realize the current code in LLD I just committed also has this issue though.) So if we make it a requirement for the caller to compensate for the prefix, it is less convenient, but also less ambiguous.

WDYT?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D67301