CodegenNameGeneratorImpl is the most complete implementation of name mangling but it is in clang::Index. This makes it very difficult to use it elsewhere in clang. This is just a NFC refactoring of CodegenNameGeneratorImpl into a class called ASTNameGenerator. Other than the name and placement change there is no functional change.
compnerd ahatanak akyrtzi aaron.ballman
- rC363878: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC).
rL363878: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC).
rG3ff8c3b73f6d: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC).
Move these into the implementation, forward declarations should be sufficient.
This isn't used, it belongs in the implementation.
Is Mangler used in the header? I think this can be in the implementation.
I think I prefer ASTNameGenerator or even ASTNameMangler (though I am partial to Microsoft's term: "decoration").
Do these still need to be public members? Back when this class was an implementation detail, that didn't matter as much, but I'd prefer a cleaner public interface before we expose this to other consumers.
Good catch! This was a struct when it was CodegenNameGeneratorImpl but these were hidden by the mere fact that they were inside the .cpp file and not in a header. Thanks!
LGTM with a few nits.
Slight preference to make this constructor explicit since it only accepts a single argument and I can't imagine wanting a converting constructor there.
Do we have to link in any new libraries in CMake for this new dependency?
I don't believe so. It builds and make check-clangs. I will try a shared lib build too. It looks like AST things are already included, and LLVM things are already included so I doubt any new library dep change is needed at all.