sbenza added inline comments. ================ Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:63 @@ +62,3 @@ + + Finder->addMatcher(returnStmt(IsBadReturnStatement, hasAncestor(IsGoodAssign)) + .bind("returnStmt"), ---------------- baloghadamsoftware wrote: > sbenza wrote: > > I dislike these uses of hasAnscestor. They are kind of slow. > > But more importantly, they break with nested functions/types. > > This particular example is not checking that the return statement is from > > the assignment operator, only that it is within it. For example, it would > > match a lambda. > > I think this would trip the check: > > > > F& operator=(const F& o) { > > std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v > 0; }); > > return *this; > > } > I can change it to hasDescendant if it is faster, but it does not solve the > lambda problem. No solution for that comes to my mind with the existing > matchers. Maybe a new matcher hasDescendantStatement could help which only > traverses statements down the AST. Is this the right way to go? hasDescendant has the same problem has hasAnscestor. I think the best is to write a matcher like:
AST_MATCHER_P(ReturnStmt, forFunction, internal::Matcher<FunctionDecl>, InnerMatcher) { ... } In there we can find the right FunctionDecl that encloses the return statement and apply the matcher. This matcher seems like a good candidate to add to ASTMatchers.h http://reviews.llvm.org/D18265 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits