[clangtidy] Allow safe suspensions in coroutine-hostile-raii check (#72954)

Certain `awaitable` types could be safe to `co_await` on even when we
have suspension-hostile RAII objects in scope.
This PR adds a way for users to mark such safe `awaitable` and silence
false positive warnings in `co_await` expressions involving such an
`awaitable`.

`co_await`-ing an expression of `awaitable` type is considered safe if
the type is part of `SafeAwaiatablesList` check option. RAII objects
persisting across such a `co_await` expression are
considered safe and hence are not flagged.

clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp

@@ -52,27 +52,42 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>,
   }
   return IsHostile;
 }
+
+// Matches the expression awaited by the `co_await`.
+AST_MATCHER_P(CoawaitExpr, awaitable, ast_matchers::internal::Matcher<Expr>,
+              InnerMatcher) {
+  if (Expr *E = Node.getCommonExpr())
+    return InnerMatcher.matches(*E, Finder, Builder);
+  return false;
+}
+
+auto typeWithNameIn(const std::vector<StringRef> &Names) {
+  return hasType(
+      hasCanonicalType(hasDeclaration(namedDecl(hasAnyName(Names)))));
+}
 } // namespace
 
 CoroutineHostileRAIICheck::CoroutineHostileRAIICheck(StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       RAIITypesList(utils::options::parseStringList(
-          Options.get("RAIITypesList", "std::lock_guard;std::scoped_lock"))) {}
+          Options.get("RAIITypesList", "std::lock_guard;std::scoped_lock"))),
+      AllowedAwaitablesList(utils::options::parseStringList(
+          Options.get("AllowedAwaitablesList", ""))) {}
 
 void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) {
   // A suspension happens with co_await or co_yield.
   auto ScopedLockable = varDecl(hasType(hasCanonicalType(hasDeclaration(
                                     hasAttr(attr::Kind::ScopedLockable)))))
                             .bind("scoped-lockable");
-  auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration(
-                               namedDecl(hasAnyName(RAIITypesList))))))
-                       .bind("raii");
-  Finder->addMatcher(expr(anyOf(coawaitExpr(), coyieldExpr()),
-                          forEachPrevStmt(declStmt(forEach(
-                              varDecl(anyOf(ScopedLockable, OtherRAII))))))
-                         .bind("suspension"),
-                     this);
+  auto OtherRAII = varDecl(typeWithNameIn(RAIITypesList)).bind("raii");
+  auto AllowedSuspend = awaitable(typeWithNameIn(AllowedAwaitablesList));
+  Finder->addMatcher(
+      expr(anyOf(coawaitExpr(unless(AllowedSuspend)), coyieldExpr()),
+           forEachPrevStmt(
+               declStmt(forEach(varDecl(anyOf(ScopedLockable, OtherRAII))))))
+          .bind("suspension"),
+      this);
 }
 
 void CoroutineHostileRAIICheck::check(const MatchFinder::MatchResult &Result) {
@@ -94,5 +109,7 @@ void CoroutineHostileRAIICheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "RAIITypesList",
                 utils::options::serializeStringList(RAIITypesList));
+  Options.store(Opts, "SafeAwaitableList",
+                utils::options::serializeStringList(AllowedAwaitablesList));
 }
 } // namespace clang::tidy::misc

clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h

@@ -43,6 +43,9 @@ private:
   // List of fully qualified types which should not persist across a suspension
   // point in a coroutine.
   std::vector<StringRef> RAIITypesList;
+  // List of fully qualified awaitable types which are considered safe to
+  // co_await.
+  std::vector<StringRef> AllowedAwaitablesList;
 };
 
 } // namespace clang::tidy::misc

clang-tools-extra/docs/clang-tidy/checks/misc/coroutine-hostile-raii.rst

@@ -29,16 +29,15 @@ Following types are considered as hostile:
 .. code-block:: c++
 
   // Call some async API while holding a lock.
-  {
-    const my::MutexLock l(&mu_);
+  task coro() {
+    const std::lock_guard l(&mu_);
 
     // Oops! The async Bar function may finish on a different
-    // thread from the one that created the MutexLock object and therefore called
-    // Mutex::Lock -- now Mutex::Unlock will be called on the wrong thread.
+    // thread from the one that created the lock_guard (and called
+    // Mutex::Lock). After suspension, Mutex::Unlock will be called on the wrong thread.
     co_await Bar();
   }
 
-
 Options
 -------
 
@@ -48,3 +47,37 @@ Options
     persist across suspension points.
     Eg: ``my::lockable; a::b;::my::other::lockable;``
     The default value of this option is `"std::lock_guard;std::scoped_lock"`.
+
+.. option:: AllowedAwaitablesList
+
+    A semicolon-separated list of qualified types of awaitables types which can
+    be safely awaited while having hostile RAII objects in scope.
+
+    ``co_await``-ing an expression of ``awaitable`` type is considered
+    safe if the ``awaitable`` type is part of this list.
+    RAII objects persisting across such a ``co_await`` expression are
+    considered safe and hence are not flagged.
+
+    Example usage:
+
+    .. code-block:: c++
+
+      // Consider option AllowedAwaitablesList = "safe_awaitable"
+      struct safe_awaitable {
+        bool await_ready() noexcept { return false; }
+        void await_suspend(std::coroutine_handle<>) noexcept {}
+        void await_resume() noexcept {}
+      };
+      auto wait() { return safe_awaitable{}; }
+
+      task coro() {
+        // This persists across both the co_await's but is not flagged
+        // because the awaitable is considered safe to await on.
+        const std::lock_guard l(&mu_);
+        co_await safe_awaitable{};
+        co_await wait();
+      }
+
+    Eg: ``my::safe::awaitable;other::awaitable``
+    The default value of this option is empty string `""`.
+

clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp

@@ -1,7 +1,8 @@
 // RUN: %check_clang_tidy -std=c++20 %s misc-coroutine-hostile-raii %t \
-// RUN:   -config="{CheckOptions: \
-// RUN:             {misc-coroutine-hostile-raii.RAIITypesList: \
-// RUN:               'my::Mutex; ::my::other::Mutex'}}"
+// RUN:   -config="{CheckOptions: {\
+// RUN:             misc-coroutine-hostile-raii.RAIITypesList: 'my::Mutex; ::my::other::Mutex', \
+// RUN:             misc-coroutine-hostile-raii.AllowedAwaitablesList: 'safe::awaitable; ::my::other::awaitable' \
+// RUN:             }}"
 
 namespace std {
 
@@ -135,6 +136,20 @@ ReturnObject scopedLockableTest() {
     absl::Mutex no_warning_5;
 }
 
+namespace safe {
+  struct awaitable {
+  bool await_ready() noexcept { return false; }
+  void await_suspend(std::coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+} // namespace safe
+ReturnObject RAIISafeSuspendTest() {
+  absl::Mutex a;
+  co_await safe::awaitable{};
+  using other = safe::awaitable;
+  co_await other{};
+} 
+
 void lambda() {
   absl::Mutex no_warning;
   auto lambda = []() -> ReturnObject {