[PATCH] D158872: [clang][ASTMatchers] Add a few type-related Matchers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Are these matchers going to be used in-tree (by clang-tidy, or something else)? We typically do not add new AST matches until there's a need for them because the AST matchers have a pretty big impact on build times of Clang itself.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3971
 
+/// Matches the type object that represents this TypeDecl.
+AST_MATCHER_P(TypeDecl, hasTypeForDecl, internal::Matcher<QualType>,
----------------
This could use some example code that demonstrates usage.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3974
+              InnerMatcher) {
+  QualType QT(Node.getTypeForDecl(), 0);
+  if (!QT.isNull())
----------------
I think this needs to go through `ASTContext::getTypeDeclType()` for correctness, as the `ASTContext` is what manages the type cache and performs extra work: https://github.com/llvm/llvm-project/blob/45eb6026d979e306a12ea9e223852b28c3c9edbf/clang/include/clang/AST/ASTContext.h#L1561


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7061
 
+/// Matches a fixed int type of a specified bit width.
+///
----------------
How about: Matches a '_BitInt' or '_ExtInt' type.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7068
+/// bitIntType()
+///   matches "_BitInt(10) i"
+extern const AstTypeMatcher<BitIntType> bitIntType;
----------------
Drop the ` i` (it only matches the type, not the whole declaration).


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7078
+/// constantMatrixType()
+///   matches the type of the typedef declaration "X".
+extern const AstTypeMatcher<ConstantMatrixType> constantMatrixType;
----------------
Seems like some type/declaration confusion here as well -- it matches the underlying type used by the typedef declaration of `X`. Same applies below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158872/new/

https://reviews.llvm.org/D158872