[PATCH] D93940: [clang-tidy] Add a check for blocking types and functions.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.cpp:1
+//===--- AsyncBlockingCheck.cpp - clang-tidy ----------------------------===//
+//
----------------
Please make length same as end of comment.


================
Comment at: clang-tools-extra/clang-tidy/concurrency/AsyncBlockingCheck.h:27
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
----------------
Please remove empty line between methods. See other checks as example. Same below.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:6
+
+Checks for some synchronous functions and types that volunteerly preempt system thread.
+Volunteer preemption of a system thread in asynchronous code
----------------
Please distribute words more evenly up to 80 characters.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:13
+The preemptive functions/types can be separated into the following categories:
+ - explicit sleep(3)-like functions
+ - sleeping/waiting synchronization primitives
----------------
See other documentation for example of lists. Same below.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:28
+  Specifies additional lock type names separated with semicolon. Usually they
+  implement C++17 BasicLockable, Lockable, TimedLockable, Mutex, or TimedMutex
+  requirement or has one or several methods from the following list:
----------------
Will be good idea to highlight `BasicLockable` and other with single back-ticks.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:30
+  requirement or has one or several methods from the following list:
+    - `lock`
+    - `try_lock_for`
----------------
Please use double beck-ticks for language constructs. Same below.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:68
+
+  Specifies whether search for std::atomic types which are not always lock-free.
+  Non-lockfree atomics use std synchronization primitives (e.g. std::mutex), so
----------------
Please highlight `std::atomic` with double back-ticks.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:69
+  Specifies whether search for std::atomic types which are not always lock-free.
+  Non-lockfree atomics use std synchronization primitives (e.g. std::mutex), so
+  they may block current system thread for a while. If such accesses are
----------------
Please highlight `std::mutex` with double back-ticks.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-async-blocking.rst:74
+
+  If set to true, checks `std::atomic<T>::is_always_lock_free` and warns about
+  `std::atomic<T>`.
----------------
Please use double beck-ticks for language constructs. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93940