[PATCH] D118370: [clang-tidy] bugprone-signal-handler: Code refactor (NFC)

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:167
+                                       const Expr *CallOrRef) {
+  const bool FunctionIsCalled = isa<CallExpr>(CallOrRef);
+
----------------
This can probably just be inlined into its only use.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:36
 private:
+  bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef);
   bool isFunctionAsyncSafe(const FunctionDecl *FD) const;
----------------
This isn't a very descriptive name, and the bool return value conveys no meaning. Probably could do with some documentation.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:46-49
   llvm::StringSet<> &ConformingFunctions;
 
   static llvm::StringSet<> MinimalConformingFunctions;
   static llvm::StringSet<> POSIXConformingFunctions;
----------------
While you're refactoring, those static StringSets really belong in the implementation file, and the ConformingFunctions should be a const ref.

However global destructores are kind of a taboo thing, Maybe there is case to change them into a sorted array and binary search them to see if a function is conforming.
Probably wouldn't advise using TableGens StringMatcher to build this functionality as that maybe a little too far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118370