[PATCH] D91164: [clang-tidy] Improve C++ support in bugprone-signal-handler.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:100-101
     const LangOptions &LangOpts) const {
-  // FIXME: Make the checker useful on C++ code.
-  if (LangOpts.CPlusPlus)
+  // FIXME: Improve C++ support.
+  if (LangOpts.CPlusPlus17)
     return false;
----------------
njames93 wrote:
> What happens differently in c++17 and later?
The plan is to add later features to the checker that check different restrictions on the signal handler function. These restrictions may differ in C++14 and C++17 ([[ https://en.cppreference.com/w/cpp/utility/program/signal | cppreference.com ]] tells about the difference too, and CERT rule MSC54-CPP). I do not want to make the checker valid for C++17 and later restrict it to C++14 again (if C++14 but not C++17 features are added). (It is not exactly clarified which aspects of the restrictions are to be checked in the checker.) In the current state the allowed callable functions seems to be the same for C++11 and later (this is defined by POSIX).


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:137
 
+  if (!HandlerDecl->isExternC()) {
+    reportNonExternCBug(HandlerExpr, HandlerDecl);
----------------
njames93 wrote:
> Why can't c++ linkage functions be used a signal handlers.
> This is also not sticking to the documentation of this check. Ths documentation just lists that it guards against non asynchronous-safe functions being called from signal handlers.
The checker documentation will be fixed. For C++14 the "common subset of C and C++" is allowed to be used as signal handler (but not for C++17) and only C linkage. For C++17 it is not clear if C++ linkage is allowed, and it is not clear if calling functions with C++ linkage is allowed from the signal handler.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:241-242
+void SignalHandlerCheck::reportLambdaBug(const Expr *HandlerRef) {
+  diag(HandlerRef->getBeginLoc(),
+       "lambda expression is not allowed as signal handler");
+}
----------------
njames93 wrote:
> Why can't lambdas be used as signal handlers, If they are stateless they can be implicitly converted to function pointers. Or is this something about needing C linkage?
I suspect that the problem is the C linkage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91164