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

Reply via email to