[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

This revision was automatically updated to reflect the committed changes. Closed by commit rG49bffa5f8b79: [clang-tidy] misc-no-recursion: a new check (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72362/new/

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri updated this revision to Diff 244509. lebedev.ri marked 8 inline comments as done. lebedev.ri added a comment. In D72362#1874690 , @aaron.ballman wrote: > LGTM aside from some nits. Thank you for the review! Nits addressed. Repository:

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some nits. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:70 + +template class SmartSmallSetVector { +public:

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:28 +// 2. it is immutable, the way it was constructed it will stay. +template class ImmutableSmartSet { + ArrayRef Vector; aaron.ballman wrote: > "Smart" is

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri updated this revision to Diff 244354. lebedev.ri marked 16 inline comments as done and an inline comment as not done. lebedev.ri added a comment. Ping. @aaron.ballman thank you for taking a look! Addressed review notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:28 +// 2. it is immutable, the way it was constructed it will stay. +template class ImmutableSmartSet { + ArrayRef Vector; "Smart" is not a descriptive

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

NoQ added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153 +// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 'bar' here: +// CHECK-NOTES: :[[@LINE-14]]:18: note: Frame #2: function 'bar' calls

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri marked an inline comment as not done. lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153 +// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 'bar' here: +// CHECK-NOTES:

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153 +// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 'bar' here: +// CHECK-NOTES: :[[@LINE-14]]:18: note: Frame #2: function 'bar' calls

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri updated this revision to Diff 242725. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Fixup disclaimer printing, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72362/new/ https://reviews.llvm.org/D72362

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

NoQ added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp:153 +// CHECK-NOTES: :[[@LINE-7]]:3: note: Frame #1: function 'boo' calls function 'bar' here: +// CHECK-NOTES: :[[@LINE-14]]:18: note: Frame #2: function 'bar' calls

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:21 + +inline bool operator==(const CallGraphNode::CallRecord , + const CallGraphNode::CallRecord ) { NoQ wrote: > lebedev.ri wrote: > >

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri updated this revision to Diff 242713. lebedev.ri marked 11 inline comments as done. lebedev.ri added a comment. @NoQ Thank you for taking a look! Rebased, addressed most of the review notes (some tests could be added), split callgraph changes into a separate review. Repository:

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

NoQ added a comment. `CallGraph` changes look nice, i don't see any immediate problems and it's a nice-to-have thing that other users may benefit from (the static analyzer wouldn't though, it's only interested in topological sorting order on the graph). I guess you could have as well stored a

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

bader added a comment. @lebedev.ri, thanks for the suggestion. We will investigate this option when we ready to upload clang diagnostics patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72362/new/ https://reviews.llvm.org/D72362

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri added a comment. Herald added a subscriber: Charusso. In D72362#1819243 , @bader wrote: > In D72362#1817682 , @lebedev.ri > wrote: > > > In D72362#1817599 ,

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

bader added a subscriber: Naghasan. bader added a comment. In D72362#1817682 , @lebedev.ri wrote: > In D72362#1817599 , @bader wrote: > > > Does it make sense to implement such diagnostics in clang Sema,

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. In D72362#1817599 , @bader wrote: > Does it make sense to implement such diagnostics in clang Sema, considering > that OpenCL does not allow recursion? > We implemented similar

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

bader added a comment. Does it make sense to implement such diagnostics in clang Sema, considering that OpenCL does not allow recursion? We implemented similar diagnostics for SYCL programming model and would be like to upstream it to clang later

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri marked 8 inline comments as done. lebedev.ri added a comment. Thanks for taking a look. Some deflective replies inline. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:21 + +inline bool operator==(const CallGraphNode::CallRecord , +

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

JonasToth added a comment. So that is were the CTU question comes from? :) Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:21 + +inline bool operator==(const CallGraphNode::CallRecord , + const CallGraphNode::CallRecord ) {

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri updated this revision to Diff 236843. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. s/1/true/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72362/new/ https://reviews.llvm.org/D72362 Files:

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri added a comment. In D72362#1808825 , @Eugene.Zelenko wrote: > It'll be reasonable to add CERT alias. I'm not sure about that. This diagnoses **any** potential recursion, while CERT is much more specific than that. (`Avoid cycles during

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

Eugene.Zelenko added a comment. It'll be reasonable to add CERT alias. Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:213 + CallGraphNode::CallRecord *Node = + while (1) { +// Did we see this node before? true

[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, ffrankies, Eugene.Zelenko, erichkeane, NoQ. lebedev.ri added a project: LLVM. Herald added subscribers: xazax.hun, Anastasia, mgorny. Herald added a project: clang. Recursion is a powerful tool, but like any