[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