Charusso 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");
----------------
abelkocsis wrote:
> 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`.
Every single check runs on the whole `TranslationUnitDecl`, and the matcher
will try to match on every of the TU's descendant, that is why it emit multiple
reports in the same single run. There is no need to use `translationUnitDecl()`
and nor to use `forEachDescendant()`.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:38
+ functionDecl(hasAnyListedName(ThreadList)))))))
+ .bind("thread")),
+ hasDescendant(varDecl(hasType(recordDecl(hasName("std::thread")))))
----------------
You can emit the binding of `thread`.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:56
+ diag(MatchedSignal->getExprLoc(),
+ "singal function should not be called in a multithreaded program");
+}
----------------
`singal` -> `signal`
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75229/new/
https://reviews.llvm.org/D75229
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
