[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis marked 2 inline comments as done.
abelkocsis added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:33-50
+void SignalInMultithreadedProgramCheck::registerMatchers(MatchFinder *Finder) {
+  auto signalCall =
+      callExpr(
+          ignoringImpCasts(hasDescendant(declRefExpr(hasDeclaration(
+              functionDecl(allOf(hasName("signal"), parameterCountIs(2),
+                                 hasParameter(0, hasType(isInteger())))))))))
+          .bind("signal");
----------------
steakhal wrote:
> I apologize for interrupting the review.
> Should we use `hasDescendant` for matching like everything in translation unit?
> 
> Wouldn't it be more performant to collect all the `signal` calls in a set and set a bool variable if a `ThreadList` function seen? 
> At the end of the analysis of the translation unit, emit warnings for each recorded `signal` call if the variable was set //(aka. we are in multithreaded environment)//.
> 
> Treat this comment rather a question since I'm not really familiar with `clang-tidy`.
I updated the checker not exactly in that way you mentioned, but I think you are right that we should not match for all `translationUnitDecl`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D75229