hokein abandoned this revision.
hokein added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3005
+/// \endcode
+AST_MATCHER(VarDecl, isStaticDataMember) {
+  return Node.isStaticDataMember();
----------------
aaron.ballman wrote:
> hokein wrote:
> > aaron.ballman wrote:
> > > How does this differ from the existing matcher 
> > > `hasStaticStorageDuration()` over a `fieldDecl()`?
> > `fieldDecl` document says that fieldDecl is "an instance of this class is 
> > created by Sema::ActOnField to
> > represent a member of a struct/union/class.". So for static class members, 
> > they should not belong to `fieldDecl` as they are not bound to class 
> > instances.
> >  
> > Technically, we can't apply `hasStaticStorageDuration()` and 
> > `isStaticStorageClass` over a `fieldDecl`.
> >  
> That's a really good point, but the question still remains: since we have 
> `hasStaticStorageDuration()` already, can we find a way to use that same 
> matcher rather than introduce a new matcher under a new name that does the 
> same thing?
> 
> This time I tested a matcher, and found that you get the correct behavior 
> from `varDecl(hasStaticStorageDuration(), hasParent(recordDecl()))`.
> 
> I think this is especially important to try to do because we have 
> `hasStaticStorageDuration()` and `isStaticStorageClass()`, so adding 
> `isStaticDataMember()` adds a third option to possibly confuse users.
Thanks for the explanations, I think it makes sense.

Previously I thought `isStaticDataMember` is an more obvious ast matcher.
`varDecl(hasStaticStorageDuration(), hasParent(cxxRecordDecl()))` and 
`varDecl(hasStaticStorageDuration(), hasDeclContext(cxxRecordDecl()), 
isDefinition())` can do the same thing. 


https://reviews.llvm.org/D25600



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to