[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2888-2890
+///   class Foo;
+///   class Bar : Foo {};
+///   class Baz : Bar {};
----------------
njames93 wrote:
> aaron.ballman wrote:
> > It seems like these aren't really part of the example?
> They are, just not directly. Shows it won't match any old base specifier.
Ah, might be good to put comments on there that say `// doesn't match` to make it more clear.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3537
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
-    hasType,
-    AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl,
-                                    CXXBaseSpecifier),
+    hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl),
     internal::Matcher<Decl>, InnerMatcher, 1) {
----------------
njames93 wrote:
> aaron.ballman wrote:
> > This is undoing a change that was just added less than two weeks ago, so I think the potential for breaking code is small. That said, can you explain why you think `hasClass` is a better approach than `hasType`?
> Yeah, as that change hasn't reached landed onto a release branch breaking code shouldn't be an issue, If it was I'd leave it in.
> 
> - `hasType` is very generic, whereas `hasClass` is specific to what a `CXXBaseSpecifier` supports.
> - It makes the matchers marginally simpler.
>   `hasDirectBase(hasType(cxxRecordDecl(hasName("Base"))))` vs `hasDirectBase(hasClass(hasName("Base")))`
> - In the documentation it also specifies that `hasClass` takes a `Matcher<CXXRecordDecl>, making it more user friendly.
FWIW, I prefer `hasType` to `hasClass`. You can inherit from things which are not a class, such as a struct (so the name is a bit of a misnomer, but not too awful), a class template (which you can't match with this interface), or a template type (which you also can't match with this interface).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81552