alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:94-99
+ std::unique_ptr<CFG> TheCFG =
+ CFG::buildCFG(nullptr, FunctionBody, &ASTCtx, Options);
+ if (!TheCFG)
+ return std::unique_ptr<ExprSequence>();
+
+ return llvm::make_unique<ExprSequence>(TheCFG.get(), &ASTCtx);
----------------
A little suggestion:
if (std::unique_ptr<CFG> TheCFG = ...)
return llvm::make_unique<ExprSequence>(TheCFG.get(), &ASTCtx);
return {};
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:128
+ return;
+ std::unique_ptr<ExprSequence> Sequence = createSequence(FunctionBody,
ASTCtx);
+
----------------
The main concern I have now is that the check may end up creating the CFG
multiple times for the same function, which may be rather expensive in certain
cases (consider a large function consisting of loops that trigger the matcher).
It's hard to predict how common is this situation in real code. Do you see how
this could be restructured to avoid unnecessary work?
================
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:179
+ }
+ CondVarNames.resize(CondVarNames.size() - 2);
+
----------------
I suggest a more straightforward version:
std::string CondVarNames;
for (const auto &CVM : CondVarMatches) {
if (!CondVarNames.empty())
CondVarNames.append(", ");
CondVarNames.append(CVM.getNodeAs<VarDecl>("condvar")->getNameAsString());
}
https://reviews.llvm.org/D40937
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
