baloghadamsoftware added inline comments.
================
Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:63
@@ +62,3 @@
+
+ Finder->addMatcher(returnStmt(IsBadReturnStatement,
hasAncestor(IsGoodAssign))
+ .bind("returnStmt"),
----------------
sbenza wrote:
> 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
Maybe I am wrong, but your proposal also seems a bottom-up matcher which is
slow. That is the reason I proposed a top-down matcher, e.g.
hasDescendantStatement or something like this which is top-down and only
traverses statements so it does not search in a lambda which is a declaration.
http://reviews.llvm.org/D18265
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
