aaron.ballman added inline comments.
================ Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3300 +/// matches `x.m()` and `p->m()`. +AST_MATCHER_P_OVERLOAD(clang::CXXMemberCallExpr, invokedAtType, + clang::ast_matchers::internal::Matcher<clang::QualType>, ---------------- ymandel wrote: > ymandel wrote: > > aaron.ballman wrote: > > > alexfh wrote: > > > > The name of the matcher doesn't tell me much. I had to carefully read > > > > the documentation to understand what is it about. I don't have a name > > > > that would raise no questions and wouldn't be too verbose at the same > > > > time, but a bit of verbosity wouldn't hurt I guess. How about > > > > `objectTypeAsWritten`? > > > Yeah, I think this would be a better name. Also, having some examples > > > that demonstrate where this behavior differs from `thisPointerType` would > > > be helpful. > > Agreed that it needs a new name, but I'm having trouble finding one I'm > > satisfied with. Here's the full description: "the type of the written > > implicit object argument". I base this phrasing on the class > > CXXMemberCallExpr's terminology. In `x.f(5)`, `x` is the implicit object > > argument, whether or not it is also implicitly surrounded by a cast. That > > is, "implicit" has two different meanings in this context. > > > > So, with that, how about `writtenObjectType`? It's close to > > `objectTypeAsWritten` but I'm hoping it makes more clear that the "written" > > part is the object not the type. > I've contrasted the behavior with thisPointerType in both of the examples. Do > you think this helps or do you want something more explicit? Here's a totally different direction: `onOrPointsToType()` ``` cxxMemberCallExpr(onOrPointsToType(hasDeclaration(cxxRecordDecl(hasName("Y"))))) ``` ================ Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3300 +/// matches `x.m()` and `p->m()`. +AST_MATCHER_P_OVERLOAD(clang::CXXMemberCallExpr, invokedAtType, + clang::ast_matchers::internal::Matcher<clang::QualType>, ---------------- aaron.ballman wrote: > ymandel wrote: > > ymandel wrote: > > > aaron.ballman wrote: > > > > alexfh wrote: > > > > > The name of the matcher doesn't tell me much. I had to carefully read > > > > > the documentation to understand what is it about. I don't have a name > > > > > that would raise no questions and wouldn't be too verbose at the same > > > > > time, but a bit of verbosity wouldn't hurt I guess. How about > > > > > `objectTypeAsWritten`? > > > > Yeah, I think this would be a better name. Also, having some examples > > > > that demonstrate where this behavior differs from `thisPointerType` > > > > would be helpful. > > > Agreed that it needs a new name, but I'm having trouble finding one I'm > > > satisfied with. Here's the full description: "the type of the written > > > implicit object argument". I base this phrasing on the class > > > CXXMemberCallExpr's terminology. In `x.f(5)`, `x` is the implicit object > > > argument, whether or not it is also implicitly surrounded by a cast. > > > That is, "implicit" has two different meanings in this context. > > > > > > So, with that, how about `writtenObjectType`? It's close to > > > `objectTypeAsWritten` but I'm hoping it makes more clear that the > > > "written" part is the object not the type. > > I've contrasted the behavior with thisPointerType in both of the examples. > > Do you think this helps or do you want something more explicit? > Here's a totally different direction: `onOrPointsToType()` > ``` > cxxMemberCallExpr(onOrPointsToType(hasDeclaration(cxxRecordDecl(hasName("Y"))))) > ``` > I think more explicit would be better. e.g., ``` cxxMemberCallExpr(invokedAtType(hasDeclaration(cxxRecordDecl(hasName("X"))))) matches 'x.m()' and 'p->m()'. cxxMemberCallExpr(on(thisPointerType(hasDeclaration(cxxRecordDecl(hasName("X")))))) matches nothing because the type of 'this' is 'Y' in both cases. ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56851/new/ https://reviews.llvm.org/D56851 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits