steven_wu added a comment.
Herald added a project: LLVM.
Sorry for following up late on the patch. Removing the reachability testing for
the exit block causes false positive for infinite loop cases like this:
void l() {
static int count = 5;
if (count >0) {
count--;
l();
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328173: Improve -Winfinite-recursion (authored by CodaFi,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43737?vs=139325&id=139408#toc
Repos
rtrieu accepted this revision.
rtrieu added a comment.
This revision is now accepted and ready to land.
Looks good. Ready to commit.
https://reviews.llvm.org/D43737
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-b
CodaFi updated this revision to Diff 139325.
https://reviews.llvm.org/D43737
Files:
lib/Sema/AnalysisBasedWarnings.cpp
test/SemaCXX/warn-infinite-recursion.cpp
Index: test/SemaCXX/warn-infinite-recursion.cpp
===
--- test/SemaCXX
rtrieu added a comment.
Two more changes, then everything is good to commit.
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:218-220
+// Found a path to the exit node without a recursive call.
+if (ExitID == Block->getBlockID())
+ return false;
Move
CodaFi updated this revision to Diff 138302.
CodaFi added a comment.
Respond to code review
https://reviews.llvm.org/D43737
Files:
lib/Sema/AnalysisBasedWarnings.cpp
test/SemaCXX/warn-infinite-recursion.cpp
Index: test/SemaCXX/warn-infinite-recursion.cpp
===
rtrieu added a comment.
> I believe you were around this code last, so can you remember why it was
> there?
Yes, that's an early exit to speed up the check. You can remove that check and
add a test case for it.
This was a little confusing for me, so let's refactor it a little. These
chang
CodaFi added a comment.
> Can you explain the new algorithm for checking recursive calls?
The idea is to de-clutter as much of the existing algorithm as possible. The
state dictionary is done away with now in favor of just not enqueueing
successors of CFG blocks that have recursive calls. Giv
rtrieu added inline comments.
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:255-257
// If the exit block is unreachable, skip processing the function.
if (cfg->getExit().pred_empty())
return;
This is likely what is causing my previous code example to
rtrieu added a comment.
Can you explain the new algorithm for checking recursive calls?
From the test, this code looks like what you are testing for, but it doesn't
trigger the warning.
void f() {
while(true) {
f();
}
}
https://reviews.llvm.org/D43737
___
CodaFi updated this revision to Diff 135827.
https://reviews.llvm.org/D43737
Files:
lib/Sema/AnalysisBasedWarnings.cpp
test/SemaCXX/warn-infinite-recursion.cpp
Index: test/SemaCXX/warn-infinite-recursion.cpp
===
--- test/SemaCXX
lebedev.ri added a comment.
Please upload patches with full diff (-U99)
Repository:
rC Clang
https://reviews.llvm.org/D43737
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
CodaFi created this revision.
CodaFi added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
Rewrites -Winfinite-recursion to remove the state dictionary and explore paths
in loops - especially infinite loops. The new check now detects recursion in
loop bodies dominated by a recursive
13 matches
Mail list logo
