[PATCH] D118224: [clang-tidy] bugprone-signal-handler improvements: display call chain

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:199-200
+    const FunctionDecl *HandlerDecl, const Expr *HandlerRef) {
+  int CallLevel = Itr.getPathLength() - 2;
+  const CallGraphNode *Caller = Itr.getPath(CallLevel + 1), *Callee = nullptr;
+  while (CallLevel >= 0) {
----------------
Do we have to worry about `CallLevel + 1` being negative here? (Is there a need for an assert?)


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:207-208
+         DiagnosticIDs::Note)
+        << cast<FunctionDecl>(Callee->getDecl())
+        << cast<FunctionDecl>(Caller->getDecl());
+    --CallLevel;
----------------
Do we have to worry about call expressions for which we cannot find the declaration (like a call through a function pointer)? (Should we add some test coverage involving a call stack with a call through a function pointer?)


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:39
   void reportBug(const FunctionDecl *CalledFunction, const Expr *CallOrRef,
-                 const CallExpr *SignalCall, const FunctionDecl *HandlerDecl);
+                 bool DirectHandler = false);
+  void reportHandlerCommon(llvm::df_iterator<clang::CallGraphNode *> Itr,
----------------
Might as well not make this an optional parameter -- no real benefit from it (there's only two call sites).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118224/new/

https://reviews.llvm.org/D118224