alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Could you run the check on llvm sources and post a summary of results here?
A few more inline comments.
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:102
+
szepet updated this revision to Diff 145160.
szepet marked 2 inline comments as done.
szepet added a comment.
Changes based on comments.
https://reviews.llvm.org/D40937
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/InfiniteLoopChe
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tidy/bugprone/InfiniteLoopCheck.h:37
+private:
+ bool updateSequence(Stmt *FunctionBody, ASTContext &ASTCtx);
+ const Stmt *PrevFunctionBody
szepet updated this revision to Diff 143949.
szepet marked 2 inline comments as done.
szepet added a comment.
Changes made based on comments.
The CFG recreating problem is handled the following (only for this check):
Always store the last visited function and its CFG* (in form of the Sequence*)
a
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 TheCFG =
+ CFG::buildCFG(nullptr, FunctionBody, &ASTCtx, Options);
+ if (!T
szepet updated this revision to Diff 141322.
szepet marked 4 inline comments as done.
szepet added a comment.
Addressed comments and readded the lost fixes.
https://reviews.llvm.org/D40937
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugp
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 loopEndingStmt() {
+ return stmt(anyOf(breakStmt(), returnStmt(), gotoStm
szepet added inline comments.
Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:121
+
+ Stmt *FunctionBody = nullptr;
+ if (ContainingLambda)
xazax.hun wrote:
> This could be pointer to const, right?
Since the createSequence uses it as a parameter for buildCFG
szepet updated this revision to Diff 141132.
szepet marked 3 inline comments as done.
szepet added a comment.
Removed to bugprone category,
skipping memberExpr cases for now in order to avoid false positives,
other small changes based on review comments.
https://reviews.llvm.org/D40937
Files:
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Please move this check to "bugprone-", since it seems to be an appropriate
category for this check.
https://reviews.llvm.org/D40937
__
xazax.hun added a comment.
I think, while the analyzer is more suitable for certain kinds of checks that
require deep analysis, it is still useful to have quicker syntactic checks that
can easily identify problems that are the results of typos or incorrectly
modified copy and pasted code. I thi
aaron.ballman added a reviewer: dcoughlin.
aaron.ballman added a subscriber: dcoughlin.
aaron.ballman added a comment.
I share the concerns that @Eugene.Zelenko brought up regarding this not being
in the analyzer. This is a path sensitive problem that I'm not certain
clang-tidy is the best home
xazax.hun added inline comments.
Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:121
+
+ Stmt *FunctionBody = nullptr;
+ if (ContainingLambda)
This could be pointer to const, right?
https://reviews.llvm.org/D40937
xazax.hun added a comment.
This does not support memberExprs as condition variables right now.
What happens if you have something like this:
struct X {
void f(int a) {
while(a < i) {
--i;
}
}
int i;
};
I think you could extend the test cases with some classes.
==
szepet added a comment.
In https://reviews.llvm.org/D40937#947760, @JVApen wrote:
> How does this check deal with atomic members?
> ...
This patch only works on integer types. So, since the atomic is a non-supported
type the check will skip that `while` loop.
https://reviews.llvm.org/D40937
Eugene.Zelenko added inline comments.
Comment at: docs/clang-tidy/checks/misc-infinite-loop.rst:6
+
+The check finds loops where none of the condition variables are updated in the
+body. This performs a very conservative check in order to avoid false positives
Pl
szepet updated this revision to Diff 125965.
szepet marked 9 inline comments as done.
szepet added a comment.
Updates based on comments.
https://reviews.llvm.org/D40937
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/InfiniteLoopCheck.cpp
clang-tidy/misc/InfiniteLoopCheck.h
clang-
martong added inline comments.
Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:105
+ return llvm::make_unique(
+ *(new ExprSequence(TheCFG.get(), &ASTCtx)));
+}
`make_unique` is a forwarding function, therefore there is no need to create an
object and th
JVApen added a comment.
How does this check deal with atomic members?
struct A {
std:: atomic a;
void f() const { while (a); }
};
https://reviews.llvm.org/D40937
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:94
+
+std::unique_ptr createSequence(Stmt *FunctionBody,
+ ASTContext &ASTCtx) {
Missing static?
Comment at: cla
Eugene.Zelenko added a comment.
In https://reviews.llvm.org/D40937#947658, @szepet wrote:
> Other note: If somebody would came up with an approach which can benefit from
> the symbolic execution, these solutions could still live happily in the
> different tools eg. UseAfterMove (tidy) and Misus
szepet added a comment.
Hi Eugen!
Good question, probably should have detailed it in the description.
This matcher based solution would not gain much benefit from the symbolic
execution provided information. (I mean, it would mean a much different type of
check on the states.)
The main problems
szepet updated this revision to Diff 125870.
szepet marked an inline comment as done.
szepet added a comment.
Updated the wording in the docs.
https://reviews.llvm.org/D40937
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/InfiniteLoopCheck.cpp
clang-tidy/misc/InfiniteLoopCheck.h
Eugene.Zelenko added a comment.
Does it really belong to Clang-tidy or Clang Analyzer is better place because
of path-ensitivity?
Please rebase from trunk.
General note: CLang-tidy use //check// in terminology, not //checker//.
Comment at: docs/ReleaseNotes.rst:63
+
+ The c
szepet created this revision.
Herald added subscribers: rnkovacs, baloghadamsoftware, whisperity, mgorny.
The checker aims to find loops where none of the condition variables are
updated in the body.
In this version it only works on integer types but the final aim is to make it
work for objects
25 matches
Mail list logo
