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