[PATCH] D138595: [llvm-cxxfilt] Support Microsoft demangling format
Tobias Hieta via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 30 11:55:01 PST 2022
thieta added a comment.
In D138595#3961248 <https://reviews.llvm.org/D138595#3961248>, @dblaikie wrote:
> In D138595#3959825 <https://reviews.llvm.org/D138595#3959825>, @thieta wrote:
>
>> In D138595#3957652 <https://reviews.llvm.org/D138595#3957652>, @dblaikie wrote:
>>
>>> Ping on this ^?
>>
>> Ah sorry - I thought I replied to this, but I just replied in my head.
>>
>> Yes, I like this idea, but it does feel like we are getting into the territory where we have to refactor the API to demangle since the demangle functions have different return values and handling.
>>
>> I am happy to do this, but I am not sure it's part of this diff. IMHO it's probably better to land this and then refactor the demangle API in its own diff.
>
> I'd rather avoid the technical debt of duplicating the is* functions only to figure out how to remove them later - the refactoring could go in before this patch to make the APIs amenable to this sort of usage.
Alright, I am fine with that. What approach would you prefer:
doing something in Demangle.cpp that wraps the current demangle functions.
Changing the different demangle functions to all use a similar api?
I am thinking we could make the functions return an optional and all take a const char* as input. Then we can return a false optional if it's not the right format or if it fails. Or we make it possible to return multiple errors so we know if it failed because of the format or something else (not sure this is that applicable since there can't be that many failure states). Any other thoughts?
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