Make `hasTypeLoc` matcher support more node types.

[steveire]

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?

[SilensAngelusNex]

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

[steveire]

In D101572#2730012, @SilensAngelusNex wrote:

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.

[SilensAngelusNex]

Added support for node types in groups #1 and #3.

[SilensAngelusNex]

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

138–150

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?

[SilensAngelusNex]

Rebase

[steveire]

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

3920

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

138–150

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>

[SilensAngelusNex]

Regenerate docs and replace is_one_of with TypeListContainsSuperOf.

[SilensAngelusNex]

Here's my branch on Github: https://github.com/SilensAngelusNex/llvm-project/tree/has-type-info

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

138–150

Perfect, thanks!

[steveire]

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?

[SilensAngelusNex]

I don't have commit access; you can go ahead and submit it.

Thanks for the review!

clang/include/clang/ASTMatchers/ASTMatchers.h

AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl,
                                     CXXBaseSpecifier),
     internal::Matcher<Decl>, InnerMatcher, 1) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
     return qualType(hasDeclaration(InnerMatcher)).matches(QT, Finder, Builder);
   return false;
 }
 
-/// Matches if the type location of the declarator decl's type matches
-/// the inner matcher.
+/// Matches if the type location of the declarator decl, temporary object
+/// expression, or functional cast expression matches the inner matcher.
 ///
 /// Given
 /// \code
 ///   int x;
 /// \endcode
 /// declaratorDecl(hasTypeLoc(loc(asString("int"))))
 ///   matches int x
-AST_MATCHER_P(DeclaratorDecl, hasTypeLoc, internal::Matcher<TypeLoc>, Inner) {
-  if (!Node.getTypeSourceInfo())
+///
+/// \code
+/// auto x = int(3);
+/// \code
+/// cxxTemporaryObjectExpr(hasTypeLoc(loc(asString("int"))))
+///   matches int(3)
+///
+/// \code
+/// struct Foo { Foo(int, int); };
+/// auto x = Foo(1, 2);
+/// \code
+/// cxxFunctionalCastExpr(hasTypeLoc(loc(asString("struct Foo"))))
+///   matches Foo(1, 2)
+AST_POLYMORPHIC_MATCHER_P(
+    hasTypeLoc,
+    AST_POLYMORPHIC_SUPPORTED_TYPES(DeclaratorDecl, CXXTemporaryObjectExpr,
+                                    CXXFunctionalCastExpr),
+    internal::Matcher<TypeLoc>, Inner) {
+  TypeSourceInfo *source = internal::GetTypeSourceInfo(Node);
+  if (source == nullptr) {
     // This happens for example for implicit destructors.
     return false;
-  return Inner.matches(Node.getTypeSourceInfo()->getTypeLoc(), Finder, Builder);
+  }
+  return Inner.matches(source->getTypeLoc(), Finder, Builder);
 }
 
 /// Matches if the matched type is represented by the given string.
 ///
 /// Given
 /// \code
 ///   class Y { public: void x(); };
 ///   void z() { Y* y; y->x(); }

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

inline QualType getUnderlyingType(const FriendDecl &Node) {
   if (const TypeSourceInfo *TSI = Node.getFriendType())
     return TSI->getType();
   return QualType();
 }
 inline QualType getUnderlyingType(const CXXBaseSpecifier &Node) {
   return Node.getType();
 }
 
+/// Unifies obtaining a `TypeSourceInfo` from different node types.
+inline TypeSourceInfo *GetTypeSourceInfo(const DeclaratorDecl &Node) {
+  return Node.getTypeSourceInfo();
+}
+inline TypeSourceInfo *GetTypeSourceInfo(const CXXTemporaryObjectExpr &Node) {
+  return Node.getTypeSourceInfo();
+}
+inline TypeSourceInfo *GetTypeSourceInfo(const CXXFunctionalCastExpr &Node) {
+  return Node.getTypeInfoAsWritten();
+}
+
 /// Unifies obtaining the FunctionProtoType pointer from both
 /// FunctionProtoType and FunctionDecl nodes..
 inline const FunctionProtoType *
 getFunctionProtoType(const FunctionProtoType &Node) {
   return &Node;
 }
 
 inline const FunctionProtoType *getFunctionProtoType(const FunctionDecl &Node) {
   return Node.getType()->getAs<FunctionProtoType>();
 }

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

EXPECT_TRUE(notMatches("using T = const int;",
                          typedefNameDecl(hasType(asString("int")))));
   EXPECT_TRUE(matches("using foo = int; using bar = foo;",
                       typedefNameDecl(hasType(asString("foo")), hasName("bar"))));
 }
 
 TEST(HasTypeLoc, MatchesDeclaratorDecls) {
   EXPECT_TRUE(matches("int x;",
                       varDecl(hasName("x"), hasTypeLoc(loc(asString("int"))))));
+  EXPECT_TRUE(matches("int x(3);",
+                      varDecl(hasName("x"), hasTypeLoc(loc(asString("int"))))));
+  EXPECT_TRUE(
+      matches("struct Foo { Foo(int, int); }; Foo x(1, 2);",
+              varDecl(hasName("x"), hasTypeLoc(loc(asString("struct Foo"))))));
 
   // Make sure we don't crash on implicit constructors.
   EXPECT_TRUE(notMatches("class X {}; X x;",
                          declaratorDecl(hasTypeLoc(loc(asString("int"))))));
 }
 
+TEST(HasTypeLoc, MatchesCXXFunctionCastExpr) {
+  EXPECT_TRUE(matches("auto x = int(3);",
+                      cxxFunctionalCastExpr(hasTypeLoc(loc(asString("int"))))));
+}
+
+TEST(HasTypeLoc, MatchesCXXTemporaryObjectExprs) {
+  EXPECT_TRUE(
+      matches("struct Foo { Foo(int, int); }; auto x = Foo(1, 2);",
+              cxxTemporaryObjectExpr(hasTypeLoc(loc(asString("struct Foo"))))));
+}
 
 TEST(Callee, MatchesDeclarations) {
   StatementMatcher CallMethodX = callExpr(callee(cxxMethodDecl(hasName("x"))));
 
   EXPECT_TRUE(matches("class Y { void x() { x(); } };", CallMethodX));
   EXPECT_TRUE(notMatches("class Y { void x() {} };", CallMethodX));
 
   CallMethodX = traverse(TK_AsIs, callExpr(callee(cxxConversionDecl())));