diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index e820cd39d83d2..a0e8700b0522b 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -52,27 +52,42 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher, } return IsHostile; } + +// Matches the expression awaited by the `co_await`. +AST_MATCHER_P(CoawaitExpr, awaitable, ast_matchers::internal::Matcher, + InnerMatcher) { + if (Expr *E = Node.getCommonExpr()) + return InnerMatcher.matches(*E, Finder, Builder); + return false; +} + +auto typeWithNameIn(const std::vector &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 diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h index a5e9cb89ef676..be925097692a4 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h @@ -43,6 +43,9 @@ class CoroutineHostileRAIICheck : public ClangTidyCheck { // List of fully qualified types which should not persist across a suspension // point in a coroutine. std::vector RAIITypesList; + // List of fully qualified awaitable types which are considered safe to + // co_await. + std::vector AllowedAwaitablesList; }; } // namespace clang::tidy::misc diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/coroutine-hostile-raii.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/coroutine-hostile-raii.rst index b8698ba3de853..a39c1853b313c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/coroutine-hostile-raii.rst +++ b/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 `""`. + diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp index 2d022e21c85d5..55a7e4b8f2954 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp +++ b/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 {