jkorous added inline comments.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553
+/// \endcode
+AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher<CXXRecordDecl>,
+ InnerMatcher) {
----------------
aaron.ballman wrote:
> 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`.
>> 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.
What I meant is that technically we can overload any `Matcher<CXXRecordDecl>`
matcher in the same fashion so having the overloaded version of `hasName` only
makes it somewhat special (and someone might argue that it'd impact consistency
of matchers composability). Anyway, I'm fine with your suggestion!
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
