[PATCH] D128314: [Clang-tidy] Fixing bugs in clang-tidy infinite-loop checker
Nathan James via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 22 03:06:16 PDT 2022
njames93 added a comment.
Can this patch be split in two, it seems strange to be fixing 2 unrelated bugs in one patch.
One fix for the ObjC nodes and another for the patch for the static local variables.
Also please can you run git clang-format over this patch.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:167-172
+ if (const Decl * Callee = Call->getDirectCallee()) {
+ const Decl * CanoCallee = Callee->getCanonicalDecl();
+
+ Callees.insert(CanoCallee);
+ } else
+ return false; // unresolved call
----------------
nit:
================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:175-180
+ if (const ObjCMethodDecl * Callee = Call->getMethodDecl()) {
+ const Decl * CanoCallee = Callee->getCanonicalDecl();
+
+ Callees.insert(CanoCallee);
+ } else
+ return false; // unresolved call
----------------
Nit: as above
================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:182-187
+ for (const Stmt * Child : StmtNode->children())
+ if (Child && populateCallees(Child, Callees))
+ continue;
+ else
+ return false;
+ return true;
----------------
Nit:
Also sometimes, some of the stmts in children can be null and that's expected, is there any reason we are disregarding that case.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:198-208
+ for (llvm::ArrayRef<CallGraphNode*>::iterator EltI = SCC.begin(),
+ EltE = SCC.end();
+ EltI != EltE; ++EltI) {
+ CallGraphNode * GNode = *EltI;
+ const Decl * CanoDecl = GNode->getDecl()->getCanonicalDecl();
+
+ containsFunc |= CanoDecl == Func;
----------------
This can be rewritten as a range for
```lang=c++
for (CallGraphNode* GNode : SCC) ...```
================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:204
+
+ containsFunc |= CanoDecl == Func;
+ overlap |= Callees.contains(CanoDecl);
----------------
================
Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:218-221
+ for (const Stmt * Child : Cond->children())
+ if (Child && hasStaticLocalVariable(Child))
+ return true;
+ return false;
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128314/new/
https://reviews.llvm.org/D128314
More information about the cfe-commits
mailing list