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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

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

> If a call site of llvm::demangle deals either exclusively Itanium mangled names, or exclusively MSVC-style mangled names. I don't see why we must have a single entry point (`llvm::demangle`) for demangling. Two entry points (for Itanium and MSVC-style) may be better. It avoids the problem that some platforms are unnecessarily affected by special rules from some particular platforms.


No, there's one single call site in the COFF linker, which right now does this:

  demangleMSVC()
  if (failedToDemangle && mingw)
      demangleItanium()

As the itanium and MSVC mangling schemes are completely orthogonal, this is just pointless, as the unified demangle function already can distinguish between the two just fine. This is what spurred this patch (requested by @rnk) to begin with.

The single entry point, `llvm::demangle`, which handles both MSVC and itanium, already exists, doing exactly this. It just happens to be more lax when it comes to itanium mangling.

> First, I'd prefer a solution that does not need to annotate call sites like: `demangle(demangleInput, /*strictPrefix=*/true)`. This is prerequisite.

So is the issue that the extra parameter is very verbose? Would `llvm::demangleStrict(demangleInput)` be better?

> Next, if there is some way to not demangle `__Z`, `___Z` or `___Z` prefixed names for ELF, that will be great.. If mingw or apple may need more underscores, a new function will look good to me. That will document their extensions to the Itanium mangling scheme. However, if doing this would increase code complexity significantly, I'm fine with using a unified interface.

The point of the patchset is to remove redundant boilerplate code in lld, when there's a general unified function in the llvm demangle library already which does this. There's no point in a new function for mingw/apple that would support more underscores, as the existing `llvm::demangle` function does it just fine. If you want the ELF linker to be very strict about demangling, we could add a `llvm::demangleStrict` that the ELF linker could use, if that's any better?

Or just keep a small wrapper in lld, like this:

  std::string demangleItanium(std::string input) {
    if (!input.startswith("_Z"))
      return input;
    return llvm::demangle(input);
  }


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

https://reviews.llvm.org/D67301