search

robotpy / robotpy-cppheaderparser

DEPRECATED: use cxxheaderparser instead
Other
123 stars 39 forks source link

Add support for decltypes #75

Open kdkavanagh opened 2 years ago

kdkavanagh commented 2 years ago

Adds support for decltypes in return types and method parameter types. Previously the parser was thrown off by the inclusion of the extra parenthesis used with decltype()

virtuald commented 2 years ago

Thanks for the contribution! It's likely that I won't have time to review until Saturday evening, but I'll do my best.

virtuald commented 2 years ago

I definitely forgot about this! I'm traveling, but will add it to my calendar for when I get home. There's nothing obviously wrong as far as I can see, but I have a larger corpus of stuff to run it against at home.

kdkavanagh commented 2 years ago

Hey @virtuald, any updates on this?

virtuald commented 2 years ago

... definitely forgot again. Adding it back to my calendar, I don't have anything urgent planned tonight so I should be able to do it then.

virtuald commented 2 years ago

This PR failed to parse one of the core RobotPy libraries that we use CppHeaderParser for. I'll dig in to see if I can isolate it.

kdkavanagh commented 2 years ago

Understood. I may be able to back out that change specifically.

Wasn't aware of cxxheaderparser - Fully supportive of moving off this codebase :). Do you feel it has at least close to feature parity with this library? If not, any idea of what's still missing?

FWIW, one thing we do in our codegen CI is run all generated code thru clang-format before comparing with expectations to ensure that formatting itself doesnt cause issues. Not sure if relevant to your stuff, just an unsolicited best practice we've found.

virtuald commented 2 years ago

cxxheaderparser does many of the same things as this (and is better at handing some C++11+ features), but intentionally doesn't do some of the weird ctypes/type resolving things that CppHeaderParser does.

It should be mostly feature complete, but some ambiguous constructs aren't quite supported yet. If you take a look at the tests, you can see what things are currently supported (and what the output looks like): https://github.com/robotpy/cxxheaderparser/tree/main/tests