aaron.ballman accepted this revision. aaron.ballman added a comment. This seems mostly reasonable to me, but @rsmith is more well-versed in this area and may have further comments. You should give him a few days to chime in before committing.
================ Comment at: include/clang/AST/DeclContextInternals.h:125-129 + bool containsInVector(const NamedDecl *D) { + assert(getAsVector() && "Must have a vector at this point"); + DeclsTy &Vec = *getAsVector(); + return is_contained(Vec, D); + } ---------------- Given that this is only called in one place and is effectively a one-liner, I'd get rid of this function entirely and replace the call below. ================ Comment at: include/clang/ASTMatchers/ASTMatchers.h:1169 +/// matches 'a'. +extern const internal::VariadicDynCastAllOfMatcher<Decl, IndirectFieldDecl> + indirectFieldDecl; ---------------- martong wrote: > aaron.ballman wrote: > > Be sure to update Registry.cpp and regenerate the AST matcher documentation > > by running clang\docs\tools\dump_ast_matchers.py. > > > > This change feels orthogonal to the rest of the patch; perhaps it should be > > split out into its own patch? > > This change feels orthogonal to the rest of the patch; perhaps it should be > > split out into its own patch? > > I agree this part could go into a separate patch, but the first use of this > new ASTMatcher is in the new unittests of this patch, so I think it fits > better to add them here. Okay, that's fair enough. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53655/new/ https://reviews.llvm.org/D53655 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits