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

Reply via email to