Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support templated lambdas #209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Swatinem
Copy link
Contributor

@Swatinem Swatinem commented Sep 8, 2020

helps with #208, but probably does not fully fix it.

This is very much WIP, as I try to wrap my head around how this parser actually works :-)
The result for lambdas is very much different from what llvm-cxxfilt

Some snippets just for my own reference:

Parser from llvm:
https://github.com/llvm/llvm-project/blob/df63eedef64d715ce1f31843f7de9c11fe1e597f/llvm/include/llvm/Demangle/ItaniumDemangle.h#L2661-L2667
https://github.com/llvm/llvm-project/blob/847299d3f00507f172097bad9dde61dfad0d355b/llvm/include/llvm/Demangle/ItaniumDemangle.h#L5388-L5442
Pretty-printer from llvm:
https://github.com/llvm/llvm-project/blob/df63eedef64d715ce1f31843f7de9c11fe1e597f/llvm/include/llvm/Demangle/ItaniumDemangle.h#L934-L986

@Swatinem Swatinem mentioned this pull request Sep 10, 2020
5 tasks
@khuey
Copy link
Collaborator

khuey commented Sep 14, 2020

The ultimate source of this construct appears to be itanium-cxx-abi/cxx-abi#85.

@khuey
Copy link
Collaborator

khuey commented Sep 14, 2020

One thing that's a bit tricky here is I believe these template parameters can be used as substitutions later (see https://github.com/llvm/llvm-project/blob/847299d3f00507f172097bad9dde61dfad0d355b/llvm/include/llvm/Demangle/ItaniumDemangle.h#L5397). We may need to implement ArgScope for ClosureTypeName or Vec<TemplateParamDecl> or something. It's not immediately obvious to me how to construct a C++ symbol that would use this.

@Swatinem Swatinem marked this pull request as ready for review February 10, 2022 14:24
@Swatinem
Copy link
Contributor Author

@khuey I rebased this and posted it for review.
I’m aware the feature is not fully implemented, though I won’t be putting in the effort to completely finish it up.
Considering that, I would argue to merge this as-is, even if it does not implement the feature fully, so we at least get back something rather than nothing at all (an Error).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants