Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
According to
llvm-project/clang/include/clang/AST$ git grep "TypeSourceInfo.*const"
there are lots of other classes with TypeSourceInfo accessors which could be supported here. Should any others be supported? Is something specific motivating adding support for these two classes and not the others?
Yes, the motivation for adding these is so I can use clang-transformer to refactor a bunch of constructor calls to call a static factory method instead.
// Before ns::Foo x(1); auto y = ns::Foo(1, 2); // After auto x = ns::Foo::Make(1); auto y = ns::Foo::Make(1, 2);
That refactor only needs hasTypeInfo to work on these three node types, but for consistency's sake I'd be happy to add overloads for others as well. I looked at the types in the results of the grep you posted; I think it would make sense for hasTypeInfo to be able to handle the classes in the first and third groups; do you think that would be reasonable?
Pretty sure these should work with hasTypeInfo.
- TypedefNameDecl (getTypeSourceInfo)
- CXXBaseSpecifier (getTypeSourceInfo)
- CXXCtorInitializer (getTypeSourceInfo)
- ObjCPropertyDecl (getTypeSourceInfo)
- ClassTemplateSpecializationDecl (getTypeAsWritten)
- CompoundLiteralExpr (getTypeSourceInfo)
- ExplicitCastExpr (getTypeInfoAsWritten)
- CXXTemporaryObjectExpr (getTypeSourceInfo)
- CXXUnresolvedConstructExpr (getTypeSourceInfo)
- TemplateArgumentLoc (getTypeSourceInfo)
These could make sense, but no other matchers exist for nodes of these types:
- VarTemplateSpecializationDecl (getTypeAsWritten)
- OffsetOfExpr (getTypeSourceInfo)
- ConvertVectorExpr (getTypeSourceInfo)
- VAArgExpr (getWrittenTypeInfo)
- CXXScalarValueInitExpr (getTypeSourceInfo)
These nodes have a method with a different name, but probably still make sense to use with hasTypeLoc.
- BlockDecl (getSignatureAsWritten)
- CXXNewExpr (getAllocatedTypeSourceInfo)
These all have a method that returns a TypeSourceInfo*, but seem like they're expressing something different and shouldn't work with hasTypeInfo.
- EnumDecl (getIntegerTypeSourceInfo)
- CXXRecordDecl (getLambdaTypeInfo) fails if not a lambda
- FriendDecl (getFriendType)
- ObjCMethodDecl (getReturnTypeSourceInfo)
- ObjCInterfaceDecl (getSuperClassTInfo)
- UnaryExprOrTypeTraitExpr (getArgumentTypeInfo) fails when arg is expr instead of type
Great work classifying those!
Yes, I think it makes sense to implement this for the 1st and 3rd groups. It might even make sense to make the method names on the classes consistent (in a prior patch) to make the matcher implementation simpler, but I'll leave it up to you to determine what's best there.
| clang/include/clang/ASTMatchers/ASTMatchersInternal.h | ||
|---|---|---|
| 169–181 | Is there a better way to express this? I was originally using template <typename... Expected> struct is_one_of {
template <typename T>
static constexpr bool value = (std::is_base_of_v<Expected, T> || ...);
};but that didn't compile because is_base_of_v and fold expressions are C++17 features. Maybe it would be better to just explicitly write out an overload of GetTypeSourceInfo for each type? | |
Please run llvm-project/clang/docs/tools/dump_ast_matchers.py to generate updated documentation for this (there is a urlopen(url) which slows things down, so I usually comment that out when running it).
Do you have this on a git clone somewhere? It's easier to take a patch and try it out from a git repo than phab.
| clang/include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3919 | This should somehow document the types it works with. You don't have to add examples for all of the nodes though. hasDeclaration is another example of a matcher supporting many different nodes. It has Usable as: at the bottom of its docs which gets parsed by dump_ast_matchers.py. Please make a similar change here to verify that the script generates docs for each type. | |
| clang/include/clang/ASTMatchers/ASTMatchersInternal.h | ||
| 169–181 | I like your c++17 solution too, but given that we have TypeListContainsSuperOf in this file used for this kind of thing, I think it makes sense to use that. I think this will work: template <typename T,
std::enable_if_t<ast_matchers::internal::TypeListContainsSuperOf<
ast_matchers::internal::TypeList<
CXXBaseSpecifier, CXXCtorInitializer, CXXTemporaryObjectExpr,
CXXUnresolvedConstructExpr, CompoundLiteralExpr, DeclaratorDecl,
ObjCPropertyDecl, TemplateArgumentLoc, TypedefNameDecl>, T>::value>
* = nullptr> | |
Here's my branch on Github: https://github.com/SilensAngelusNex/llvm-project/tree/has-type-info
| clang/include/clang/ASTMatchers/ASTMatchersInternal.h | ||
|---|---|---|
| 169–181 | Perfect, thanks! | |
This looks good to me. I tried running it in clang-query, which crashes with a matcher like
m binaryOperator(hasEitherOperand(hasTypeLoc(loc(asString("int")))))but it also crashes before your change, so it must be unrelated.
Do you have a commit account or will I commit this for you with the name/email in your github commit?
This should somehow document the types it works with. You don't have to add examples for all of the nodes though. hasDeclaration is another example of a matcher supporting many different nodes. It has Usable as: at the bottom of its docs which gets parsed by dump_ast_matchers.py. Please make a similar change here to verify that the script generates docs for each type.