balazske added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:146 + if (!HandlerDecl->hasBody()) { + checkFunction(HandlerDecl, HandlerExpr); return; ---------------- LegalizeAdulthood wrote: > Why do we ignore the return value of `checkFunction` here? > > Also, the name doesn't reveal to me why the result is true > or false, e.g. it acts like a predicate but its name doesn't ask > a question. The function returns if a problem (warning) was found. At the next call site it is used to display additional notes, but here it is not needed. A better name can be `isFunctionValidSignalHandler` but the function checks not only a fact, it generates the warnings too. ================ Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:151 CallGraphNode *HandlerNode = CG.getNode(HandlerDecl); - // Signal handler can be external but not unsafe, no call graph in this case. - if (!HandlerNode) - return; + assert(HandlerNode && + "Handler has body, should be present in the call graph."); ---------------- There are problems with this approach in C++ code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118370/new/ https://reviews.llvm.org/D118370 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits