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, 
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(", ");
​  }

cfe-commits mailing list

Reply via email to