djasper added inline comments.
================
Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:28
stmt(forEach(
- ifStmt(hasThen(stmt(
+ ifStmt(unless(hasParent(ifStmt())),
+ hasThen(stmt(
----------------
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?
https://reviews.llvm.org/D26125
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
