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