[PATCH] D50586: [Demangle] Add itaniumFindTypesInMangledName()

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

In https://reviews.llvm.org/D50586#1197041, @sgraenitz wrote:

> > Doing things this way, would require adding more hooks to the demangler, which does not seem particularly nice. OTOH, exposing the AST would allow other interesting uses too. ...
>
> Yes, I am totally with you. Doing this on an exposed AST adds lots of potential, but it also seems like a bigger functional change to me, which requires proper planning and feature discussions. Thus, I am in favour of doing things step by step here. I'd first mimic the old LLDB behaviour with the new demangler and remove FastDemangle from LLDB. Afterwards we can plan and discuss at which places and for which extended functionality we can make use of an exposed AST.
>
> In the meantime, I think `llvm::itaniumFindTypesInMangledName()` even adds value on its own as it's a simple way to visit parameter types in a mangled name.
>
> What do you think?


I think that's up to Erik mostly. Doing things this way definitely makes LLDB's life easier, but OTOH, I am generally trying to avoid cleaning up LLDB by pushing the mess into LLVM. This is not a particularly large mess, but it still a very odd specialized api that will only ever be used by a single customer.

I agree that the AST thing is a much bigger change, and originally I was planning to send an email to llvm-dev first (and probably I will at some point) after I clean things up, but your patches kinda forced my hand (I thought it was important for you to know that this is not a one-off thing, but there are other use cases for transforming mangled names). FWIW, I am fine with this going in and then being backed out once the more general framework is in, or just poking another hole for constructors if exposing the AST does not seem a good idea. That's particularly true as I cannot make much progress on that patch this week (though if either of you wants to pick that up, I wouldn't be offended :)).


https://reviews.llvm.org/D50586