idlecode marked an inline comment as done. idlecode added inline comments.
================ Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:28 stmt(forEach( - ifStmt(hasThen(stmt( + ifStmt(unless(hasParent(ifStmt())), + hasThen(stmt( ---------------- djasper wrote: > idlecode wrote: > > djasper wrote: > > > I think this now effectively does: > > > > > > stmt(forEach(ifStmt(unless(hasParent(ifSttmt())), ...) > > > > > > I think that's equivalent to: > > > > > > stmt(unless(ifStmt()), forEach(ifStmt(...))) > > Apparently you are right - does that mean that this matcher actually > > matches node above ifStmt (and just binds ifStmt)? > > If so maybe such expression would suffice? (it passes tests) > > ``` > > Finder->addMatcher( // note lack of stmt(forEach( > > ifStmt(unless(hasParent(ifStmt())), > > hasThen(stmt( > > anyOf(ControlFlowInterruptorMatcher, > > > > compoundStmt(has(ControlFlowInterruptorMatcher))))), > > hasElse(stmt().bind("else"))) > > .bind("if"), > > this); > > > > ``` > > > > But I'm not sure if this would be any better than your version. > Yeah. That makes sense to me. The difference is that we would start binding > ifStmts() that are direct children of declarations. But I don't know whether > we have excluded those intentionally. Do any tests break if you make this > change? I have just ran `make check-clang-tools` on updated source tree - no new failures. Upload the change? What do you mean by 'if statement as children of declaration'? (I'm new to compilers and just curious :) ) https://reviews.llvm.org/D26125 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits