aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp:22
+    /* C++ std */
+    "::std::async", //
+
----------------
The trailing comment markers don't really add much.


================
Comment at: 
clang-tools-extra/clang-tidy/concurrency/AsyncNoNewThreadsCheck.cpp:33
+
+    /* Linux */
+    "::fork",     //
----------------
If we're going to add these, we should probably also add ones for Win32 and Mac 
OS as well, like `CreateThread`, `CreateRemoteThread`, `_beginthread`, 
`_beginthreadex`, etc.


================
Comment at: 
clang-tools-extra/docs/clang-tidy/checks/concurrency-async-no-new-threads.rst:8-10
+functions and types. E.g. if the code uses C++ coroutines, it is expected
+that only new coroutines or coroutine-based primitives are created
+instead of heavy system threads.
----------------
FWIW, this suggests to me that what you really want is a way for APIs to opt 
into this behavior. There's no reason why you wouldn't have a complex system 
that has both threads and coroutines in it, but it does stand to reason that 
you may want to say "this function, and everything called within this function, 
should not create any system threads" in some situations.

The note below helps call out the expectations from the check, but it requires 
the developer to restructure the way they write code pretty drastically in 
order to make the checking behavior more reasonable, which does not seem ideal.


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

https://reviews.llvm.org/D94622

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to