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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits