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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits