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