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 &LHS,
+ const CallGraphNode::CallRecord &RHS) {
----------------
JonasToth wrote:
> That should be in the `CallGraph` code, adding a private operator overload
> does not feel right.
I didn't want to put this into callgraph, because this
cements the fact that we do not care about sourceloc,
which may not be true for other users.
I can put it there, but if told so by it's code owners (@NoQ ?)
================
Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:31
+
+// Specialize DenseMapInfo for clang::CallGraphNode::CallRecord.
+template <> struct DenseMapInfo<clang::CallGraphNode::CallRecord> {
----------------
JonasToth wrote:
> That stuff, too.
(same reasoning)
================
Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:67
+// 2. it is immutable, the way it was constructed it will stay.
+template <typename T, unsigned SmallSize> class ImmutableSmartSet {
+ ArrayRef<T> Vector;
----------------
JonasToth wrote:
> Why `smart`?
Because if it's just named `ImmutableSet`, why should it not just be a `using
ImmutableSet = DenseSet; const ImmutableSet ......`?
This denotes it's different behaviour when in small-size mode and in
non-small-size mode.
================
Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:73
+
+ bool isSmall() const { return Set.empty(); }
+
----------------
JonasToth wrote:
> That method name looks confusing. `isEmpty()`? If not, why?
See how it's used, e.g. in `count()`, see `SmallSet` also.
================
Comment at: clang-tools-extra/clang-tidy/misc/NoRecursionCheck.cpp:242
+ Decl *D = N->getDecl();
+ diag(D->getLocation(), "function '%0' is part of call graph loop")
+ << cast<NamedDecl>(D)->getName();
----------------
JonasToth wrote:
> That should be a `Note:`
I'm intentionally emitting it as a warning.
If i `// NOLINT` the main warning, does that not silence the related `Notes`?
I don't want `NOLINT`ing the "main" function to silence the report for the rest
of the functions in SCC.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72362/new/
https://reviews.llvm.org/D72362
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
