[PATCH] D138595: [llvm-cxxfilt] Support Microsoft demangling format

Tobias Hieta via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 23 23:47:14 PST 2022


thieta added a comment.

In D138595#3948378 <https://reviews.llvm.org/D138595#3948378>, @dblaikie wrote:

>> New code should probably use the demangle() function that specifies what format should be used.
>
> Not sure I agree with this direction - I'd think that callers that know which format they want to use can/should use APIs that already implement only that format, rather than passing an enum and then switching over the enum to get back to the statically known kind?
> So I'd have figured llvm-cxxfilt could have the switch-over-mangling-kind, but the lower level APIs in libDemangle wouldn't have an enum-based API, instead having dedicated functions for different demanglings. (though, arguably, the presence of `demangleNonWindows` goes against that design/direction)

I guess I can see your point. I think the problem I was running into when trying to get cxxfilt to demangle Microsoft symbols was that it was most of the API's where behaving differently and I was not able to use the common function since I was not able to limit the demanglers used if you wanted to specify the format. My instinct was to make the shared API easier and more flexible, but maybe this code should just live in cxxfilt tool instead.

> Depends if most callers are likely to know their mangling or not? Got a sense of that, or what your plans are for this new API, beyond llvm-cxxfilt?

I looked around at other uses for demangle() and the specific demangle functions:

- llvm::demangle() is used in CodeGenAction, MarkupFilter, ASAN and llvm-cxxfilt, where only cxxfilt seems to care about the demangling format.
- itaniumDemangle() is used all over the source code
- dlang and rust demangle functions are only called by cxxfilt and lldb.
- microsoftDemangle() is used in cxxfilt and undname (which is cxxfilt for windows).
- nonMicrosoftDemangle() is used in cxxfilt, llvm-nm and symbolize

With this information - I think I am in your camp here, better to move the format enum switching to cxxfilt. We can always revisit and rework the common API if more people have a use for it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138595



More information about the llvm-commits mailing list