[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