aaron.ballman added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:169
"bugprone-terminating-continue");
+ CheckFactories.registerCheck<ThreadCanceltypeAsynchronousCheck>(
+ "bugprone-thread-canceltype-asynchronous");
----------------
I think this check is likely better surfaced in the `concurrency` module. WDYT?
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp:29
+
+static Preprocessor *PP;
+
----------------
Any particular reason to use a global variable here rather than making it a
member of `ThreadCanceltypeAsynchronousCheck`?
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp:34
+ if (!PthreadCancelAsynchronousValue) {
+ const auto IsAsynchronous = [](const auto &KeyValue) -> bool {
+ return KeyValue.first->getName() == "PTHREAD_CANCEL_ASYNCHRONOUS" &&
----------------
We don't use top-level `const` on local value types (here and elsewhere in the
patch).
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp:38
+ };
+ const auto TryExpandAsInteger =
+ [](Preprocessor::macro_iterator It) -> Optional<unsigned> {
----------------
Do we know that POSIX implementations always use a simple integer here, or do
we have to worry about code like:
```
#define PTHREAD_CANCEL_ASYNCHRONOUS (1 << 0)
```
or
```
#define PTHREAD_CANCEL_ASYNCHRONOUS SOME_OTHER_MACRO
```
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp:68
+ diag(MatchedExpr->getBeginLoc(),
+ "asynchronous cancelability type should not be used");
+ }
----------------
How about: `the cancel type for a pthread should not be
'PTHREAD_CANCEL_ASYNCHRONOUS'`?
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst:9
+(``PTHREAD_CANCEL_ASYNCHRONOUS``) is generally unsafe, use type
+``PTHREAD_CANCEL_DEFERRED`` instead which is the default. Even with deferred
+cancellation, a cancellation point in an asynchronous signal handler may still
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96719/new/
https://reviews.llvm.org/D96719
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
