[PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Samuel Benzaquen via cfe-commits]

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