alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:23-25
+static internal::Matcher<Stmt> loopEndingStmt() {
+ return stmt(anyOf(breakStmt(), returnStmt(), gotoStmt(), cxxThrowExpr()));
+}
----------------
This doesn't have to be a function. It can be a local variable or can be
inlined into the callsite.
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:32
+void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) {
+ const auto loopCondition = allOf(hasCondition(expr().bind("condition")),
+ anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
----------------
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> Variable names should be nouns (as they represent state). The name should be
> camel case, and start with an upper case letter (e.g. Leader or Boats).
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:105
+ return llvm::make_unique<ExprSequence>(
+ *(new ExprSequence(TheCFG.get(), &ASTCtx)));
+}
----------------
Why not just `llvm::make_unique<ExprSequence>(TheCFG.get(), &ASTCtx)`?
It also may be that you somehow lost a bunch of fixes, since this comment has
already been addressed at some point:
https://reviews.llvm.org/D40937?id=125870#inline-357357
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:186
+ diag(LoopStmt->getLocStart(),
+ "%plural{1:The condition variable|:None of the condition variables}0 "
+ "(%1) %plural{1:is not|:are}0 updated in the loop body")
----------------
Clang diagnostics (and clang-tidy warnings) are not complete sentences and
shouldn't start with a capital letter.
https://reviews.llvm.org/D40937
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
