[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

Balázs Kéri via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 22 01:31:32 PDT 2022


balazske added inline comments.
Herald added a project: All.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:153
+    diag(HandlerLambda->getBeginLoc(),
+         "lambda function is not allowed as signal handler (until C++17)")
+        << HandlerLambda->getSourceRange();
----------------
whisperity wrote:
> I am trying to find some source for this claim but so far hit a blank. 😦 Could you please elaborate where this information comes from? Most notably, [[ https://en.cppreference.com/w/cpp/utility/program/signal | std::signal on CppReference ]] makes no mention of this, which would be the first place I would expect most people to look at.
> 
> Maybe this claim is derived from the rule that signal handlers **MUST** have C linkage? (If so, is there a warning against people setting C++-linkage functions as signal handlers in this check in general?)
This check is made to comply with a CERT rule [[ https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC54-CPP.+A+signal+handler+must+be+a+plain+old+function | MSC54-CPP ]]. According to this only the subset of C and C++ should be used in a signal handler, and it should have extern "C" linkage. This does not allow lambda functions because the linkage restriction (if not others). (From C++17 on other rules apply, this check is not applicable for such code.)


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:54
+                     std::function<void(bool)> ChainReporter);
+  /// Similar as `checkFunction` but only check for C++14 rules.
+  bool checkFunctionCPP14(const FunctionDecl *FD, const Expr *CallOrRef,
----------------
whisperity wrote:
> (Nit: to keep consistency with others and to ensure documentation renders properly, use `@p checkFunction` instead of backtick.)
I see that the backtick character is used at other places. It should work with Doxygen. The `\c` is the normal Doxygen command to make code-style text format, `\p` is used for function parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996



More information about the cfe-commits mailing list