[PATCH] D118996: [clang-tidy] Support C++14 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:328
     const LangOptions &LangOpts) const {
-  // FIXME: Make the checker useful on C++ code.
-  if (LangOpts.CPlusPlus)
-    return false;
-
-  return true;
+  return LangOpts.CPlusPlus17 == 0;
 }
----------------
whisperity wrote:
> Aren't these `bool`s?
This is a 1-bit bit field member.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:472
+    StringRef Name = FoundS->getStmtClassName();
+    if (Name.startswith("CXX")) {
+      SourceRange R = getSourceRangeOfStmt(FoundS, Ctx);
----------------
njames93 wrote:
> This just feels very hacky and it's non exhaustive, would lambda for a start as that isn't prefixed internally with CXX.
This is somewhat "hacky" but is more future-proof if new `Stmt` classes are added. I extend the list with `isa` checks for classes in **ExprCXX.h** and **StmtCXX.h**. But it is likely that when a new C++-only node is added this list is not updated (maybe no problem because new things are for later than C++17).


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:126
     const LangOptions &LangOpts) const {
-  // FIXME: Make the checker useful on C++ code.
-  if (LangOpts.CPlusPlus)
+  if (LangOpts.CPlusPlus17)
     return false;
----------------
LegalizeAdulthood wrote:
> njames93 wrote:
> > Is this check valid on Objective-C code?
> `return LangOpts.CPlusPlus17 == 0;`
I do not know if the check is applicable to Objective-C, it may depend on if the C standard (or POSIX) is applicable to Objective-C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996