aaron.ballman added inline comments.
================ Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher<CXXRecordDecl>, + InnerMatcher) { ---------------- njames93 wrote: > jkorous wrote: > > aaron.ballman wrote: > > > jkorous wrote: > > > > Nit: while "[base specifier] `hasType`" sounds natural to me for some > > > > reason `hasClass` doesn't. English is not my first language though. > > > I agree that `hasClass` seems unnatural here. Out of curiosity, could we > > > modify the `hasName` matcher to work on base specifiers so you can write: > > > `cxxRecordDecl(hasAnyBase(hasName("Base")))` as shorthand for the more > > > wordy version > > > `cxxRecordDecl(hasAnyBase(hasType(cxxRecordDecl(hasName("Base")))))`? > > Wouldn't it be strange to treat `hasName` differently than all the other > > narrowing matchers? Honest question - I feel that `hasName` might be the > > most commonly used, just don't know if that's enough to justify this. > > https://clang.llvm.org/docs/LibASTMatchersReference.html#narrowing-matchers > Repurposing `hasName` would be a pain especially considering 99% of its use > cases wont be for base class matching. > Wouldn't it be strange to treat hasName differently than all the other > narrowing matchers? Honest question - I feel that hasName might be the most > commonly used, just don't know if that's enough to justify this. > https://clang.llvm.org/docs/LibASTMatchersReference.html#narrowing-matchers Different how? I'm suggesting to overload `hasName` to work on a `CXXBaseSpecifier` since those have a name. > Repurposing hasName would be a pain especially considering 99% of its use > cases wont be for base class matching. I'm asking what the right API is for users, though, which is a bit different. Base specifiers have names (there are no unnamed base specifiers), so to me, it makes more sense for `hasName` to work with them directly since that is the thing that does name matching. I think you can accomplish this by using a `PolymorphicMatcherWithParam1` like we do for `hasOverloadedOperatorName` which can narrow to either a `CXXOperatorCallExpr` or a `FunctionDecl`. ================ Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3621 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClassOrClassTemplate, + internal::Matcher<CXXRecordDecl>, InnerMatcher) { ---------------- jkorous wrote: > I think we should just use `eachOf` matcher for this kind of composition. > > https://clang.llvm.org/docs/LibASTMatchersReference.html#traversal-matchers +1 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