https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/72954
>From c863646669d0b2b54e1c1c353b063a8209730528 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 06:51:48 +0100 Subject: [PATCH 01/11] [clangtidy]Allow safe suspensions in coroutine-hostile-raii check --- .../misc/CoroutineHostileRAIICheck.cpp | 17 ++++++++++++++++- .../checkers/misc/coroutine-hostile-raii.cpp | 11 +++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index e820cd39d83d21b..1c39a6624a1da69 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -52,6 +52,19 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>, } return IsHostile; } + +AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>, + InnerMatcher) { + return Node.getCommonExpr() && + InnerMatcher.matches(*Node.getCommonExpr(), Finder, Builder); +} + +AST_MATCHER(Decl, isRAIISafeAwaitable) { +for (const auto &Attr : Node.specific_attrs<clang::AnnotateAttr>()) + if (Attr->getAnnotation() == "coro_raii_safe_suspend") + return true; + return false; +} } // namespace CoroutineHostileRAIICheck::CoroutineHostileRAIICheck(StringRef Name, @@ -68,7 +81,9 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) { auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration( namedDecl(hasAnyName(RAIITypesList)))))) .bind("raii"); - Finder->addMatcher(expr(anyOf(coawaitExpr(), coyieldExpr()), + auto Allowed = awaiatable( + hasType(hasCanonicalType(hasDeclaration(isRAIISafeAwaitable())))); + Finder->addMatcher(expr(anyOf(coawaitExpr(unless(Allowed)), coyieldExpr()), forEachPrevStmt(declStmt(forEach( varDecl(anyOf(ScopedLockable, OtherRAII)))))) .bind("suspension"), 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 2d022e21c85d566..4a1dc61b34ded18 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 @@ -62,6 +62,12 @@ struct suspend_always { }; } // namespace std +struct [[clang::annotate("coro_raii_safe_suspend")]] raii_safe_suspend { + bool await_ready() noexcept { return false; } + void await_suspend(std::coroutine_handle<>) noexcept {} + void await_resume() noexcept {} +}; + struct ReturnObject { struct promise_type { ReturnObject get_return_object() { return {}; } @@ -135,6 +141,11 @@ ReturnObject scopedLockableTest() { absl::Mutex no_warning_5; } +ReturnObject RAIISafeSuspendTest() { + absl::Mutex a; + co_await raii_safe_suspend{}; +} + void lambda() { absl::Mutex no_warning; auto lambda = []() -> ReturnObject { >From b35e1fece9b35f5faf1a28a5fa744ed29ddbf250 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 07:03:16 +0100 Subject: [PATCH 02/11] fix format --- clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index 1c39a6624a1da69..235bb584be14754 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -60,7 +60,7 @@ AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>, } AST_MATCHER(Decl, isRAIISafeAwaitable) { -for (const auto &Attr : Node.specific_attrs<clang::AnnotateAttr>()) + for (const auto &Attr : Node.specific_attrs<clang::AnnotateAttr>()) if (Attr->getAnnotation() == "coro_raii_safe_suspend") return true; return false; >From 9d1c6d90e3fd7b4eb673bd67ccbf48c1d24ce329 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 07:06:51 +0100 Subject: [PATCH 03/11] rename var --- .../clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index 235bb584be14754..6b438b6c44d855c 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -81,13 +81,14 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) { auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration( namedDecl(hasAnyName(RAIITypesList)))))) .bind("raii"); - auto Allowed = awaiatable( + auto SafeSuspend = awaiatable( hasType(hasCanonicalType(hasDeclaration(isRAIISafeAwaitable())))); - Finder->addMatcher(expr(anyOf(coawaitExpr(unless(Allowed)), coyieldExpr()), - forEachPrevStmt(declStmt(forEach( - varDecl(anyOf(ScopedLockable, OtherRAII)))))) - .bind("suspension"), - this); + Finder->addMatcher( + expr(anyOf(coawaitExpr(unless(SafeSuspend)), coyieldExpr()), + forEachPrevStmt( + declStmt(forEach(varDecl(anyOf(ScopedLockable, OtherRAII)))))) + .bind("suspension"), + this); } void CoroutineHostileRAIICheck::check(const MatchFinder::MatchResult &Result) { >From 37f412080ee64d98527aeb1a06429846d935de81 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 07:40:13 +0100 Subject: [PATCH 04/11] more work --- .../clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 6 +++--- .../checkers/misc/coroutine-hostile-raii.cpp | 11 +++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index 6b438b6c44d855c..ee7565d5c714d4d 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -59,7 +59,7 @@ AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>, InnerMatcher.matches(*Node.getCommonExpr(), Finder, Builder); } -AST_MATCHER(Decl, isRAIISafeAwaitable) { +AST_MATCHER(Decl, isRAIISafe) { for (const auto &Attr : Node.specific_attrs<clang::AnnotateAttr>()) if (Attr->getAnnotation() == "coro_raii_safe_suspend") return true; @@ -81,8 +81,8 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) { auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration( namedDecl(hasAnyName(RAIITypesList)))))) .bind("raii"); - auto SafeSuspend = awaiatable( - hasType(hasCanonicalType(hasDeclaration(isRAIISafeAwaitable())))); + auto SafeSuspend = + awaiatable(hasType(hasCanonicalType(hasDeclaration(isRAIISafe())))); Finder->addMatcher( expr(anyOf(coawaitExpr(unless(SafeSuspend)), coyieldExpr()), forEachPrevStmt( 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 608f9aec6ba0a71..6fd649bb021b414 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 @@ -62,12 +62,6 @@ struct suspend_always { }; } // namespace std -struct [[clang::annotate("coro_raii_safe_suspend")]] raii_safe_suspend { - bool await_ready() noexcept { return false; } - void await_suspend(std::coroutine_handle<>) noexcept {} - void await_resume() noexcept {} -}; - struct ReturnObject { struct promise_type { ReturnObject get_return_object() { return {}; } @@ -141,6 +135,11 @@ ReturnObject scopedLockableTest() { absl::Mutex no_warning_5; } +struct [[clang::annotate("coro_raii_safe_suspend")]] raii_safe_suspend { + bool await_ready() noexcept { return false; } + void await_suspend(std::coroutine_handle<>) noexcept {} + void await_resume() noexcept {} +}; ReturnObject RAIISafeSuspendTest() { absl::Mutex a; co_await raii_safe_suspend{}; >From e438500e8775887f72f234395a781da66277c803 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 08:02:46 +0100 Subject: [PATCH 05/11] add docs --- .../checks/misc/coroutine-hostile-raii.rst | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) 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 b8698ba3de85300..b36bded6866641b 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,15 +29,28 @@ 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(); } +.. code-block:: c++ + + struct [[clang::annotate("coro_raii_safe_suspend")]] safe_awaitable { + bool await_ready() noexcept { return false; } + void await_suspend(std::coroutine_handle<>) noexcept {} + void await_resume() noexcept {} + }; + + task coro() { + const std::lock_guard l(&mu_); + co_await safe_awaitable{}; + } + Options ------- >From 5a3af35ee3f17b384dd6314e5ea1918160f19856 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 09:00:39 +0100 Subject: [PATCH 06/11] update docs (cherry picked from commit f2092a62148dca3ee6cb3ea7a80fc7c9ad640f50) --- .../checks/misc/coroutine-hostile-raii.rst | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 b36bded6866641b..93e9158d9f3fd16 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 @@ -38,6 +38,22 @@ Following types are considered as hostile: co_await Bar(); } +Exclusions +------- +It is possible to make the check treat certain suspensions as safe. +``co_await``-ing an expression of ``awaitable`` type is considered +safe if the ``awaitable`` type is annotated with +``[[clang::annotate("coro_raii_safe_suspend")]]``. +RAII objects persisting across such a ``co_await`` expression are +considered safe and hence are not flagged. + +This annotation can be used to mark ``awaitable`` types which can be safely +awaited while having hostile RAII objects in scope. For example, such safe +``awaitable`` could ensure resumption on the same thread or even unlock the mutex +on suspension and reacquire on resumption. + +Example usage: + .. code-block:: c++ struct [[clang::annotate("coro_raii_safe_suspend")]] safe_awaitable { @@ -51,6 +67,13 @@ Following types are considered as hostile: co_await safe_awaitable{}; } + auto wait() { return safe_awaitable{}; } + + task coro() { + const std::lock_guard l(&mu_); // No warning. + co_await safe_awaitable{}; + co_await wait(); + } Options ------- >From 5f249bab1ada4f362bff0a269bd2bd866ef849ba Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 09:26:15 +0100 Subject: [PATCH 07/11] add comment for matchers --- .../clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index ee7565d5c714d4d..838d2f5667cc210 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -53,12 +53,15 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>, return IsHostile; } +// Matches the expression awaited by the `co_await`. AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>, InnerMatcher) { return Node.getCommonExpr() && InnerMatcher.matches(*Node.getCommonExpr(), Finder, Builder); } +// Matches a declaration annotated with +// [[clang::annotate("coro_raii_safe_suspend")]]. AST_MATCHER(Decl, isRAIISafe) { for (const auto &Attr : Node.specific_attrs<clang::AnnotateAttr>()) if (Attr->getAnnotation() == "coro_raii_safe_suspend") >From 3c62c83de88d80797fe216287ba5633213658353 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 13:09:31 +0100 Subject: [PATCH 08/11] use clangtidy options instead of annotation --- .../misc/CoroutineHostileRAIICheck.cpp | 19 ++---- .../misc/CoroutineHostileRAIICheck.h | 3 + .../checks/misc/coroutine-hostile-raii.rst | 67 +++++++++---------- .../checkers/misc/coroutine-hostile-raii.cpp | 15 +++-- 4 files changed, 52 insertions(+), 52 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index 838d2f5667cc210..3a98db34b3a887d 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -59,22 +59,15 @@ AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>, return Node.getCommonExpr() && InnerMatcher.matches(*Node.getCommonExpr(), Finder, Builder); } - -// Matches a declaration annotated with -// [[clang::annotate("coro_raii_safe_suspend")]]. -AST_MATCHER(Decl, isRAIISafe) { - for (const auto &Attr : Node.specific_attrs<clang::AnnotateAttr>()) - if (Attr->getAnnotation() == "coro_raii_safe_suspend") - return true; - return false; -} } // 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"))), + SafeAwaitablesList(utils::options::parseStringList( + Options.get("SafeAwaitablesList", ""))) {} void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) { // A suspension happens with co_await or co_yield. @@ -84,8 +77,8 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) { auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration( namedDecl(hasAnyName(RAIITypesList)))))) .bind("raii"); - auto SafeSuspend = - awaiatable(hasType(hasCanonicalType(hasDeclaration(isRAIISafe())))); + auto SafeSuspend = awaiatable(hasType(hasCanonicalType( + hasDeclaration(namedDecl(hasAnyName(SafeAwaitablesList)))))); Finder->addMatcher( expr(anyOf(coawaitExpr(unless(SafeSuspend)), coyieldExpr()), forEachPrevStmt( @@ -113,5 +106,7 @@ void CoroutineHostileRAIICheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "RAIITypesList", utils::options::serializeStringList(RAIITypesList)); + Options.store(Opts, "SafeAwaitableList", + utils::options::serializeStringList(SafeAwaitablesList)); } } // 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 a5e9cb89ef67695..3821e255a81c2c0 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<StringRef> RAIITypesList; + // List of fully qualified awaitable types for which are considered safe to + // co_await. + std::vector<StringRef> SafeAwaitablesList; }; } // 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 93e9158d9f3fd16..4e99ebc7b5c4cb7 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 @@ -38,49 +38,46 @@ Following types are considered as hostile: co_await Bar(); } -Exclusions +Options ------- -It is possible to make the check treat certain suspensions as safe. -``co_await``-ing an expression of ``awaitable`` type is considered -safe if the ``awaitable`` type is annotated with -``[[clang::annotate("coro_raii_safe_suspend")]]``. -RAII objects persisting across such a ``co_await`` expression are -considered safe and hence are not flagged. -This annotation can be used to mark ``awaitable`` types which can be safely -awaited while having hostile RAII objects in scope. For example, such safe -``awaitable`` could ensure resumption on the same thread or even unlock the mutex -on suspension and reacquire on resumption. +.. option:: RAIITypesList -Example usage: + A semicolon-separated list of qualified types which should not be allowed to + 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"`. -.. code-block:: c++ +.. option:: SafeAwaitablesList - struct [[clang::annotate("coro_raii_safe_suspend")]] safe_awaitable { - bool await_ready() noexcept { return false; } - void await_suspend(std::coroutine_handle<>) noexcept {} - void await_resume() noexcept {} - }; + A semicolon-separated list of qualified types of awaitables types which can + be safely awaited while having hostile RAII objects in scope. - task coro() { - const std::lock_guard l(&mu_); - co_await safe_awaitable{}; - } + ``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. - auto wait() { return safe_awaitable{}; } + Example usage: - task coro() { - const std::lock_guard l(&mu_); // No warning. - co_await safe_awaitable{}; - co_await wait(); - } + .. code-block:: c++ -Options -------- + // Cosnider option SafeAwaitablesList = "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{}; } -.. option:: RAIITypesList + 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 `""`. - A semicolon-separated list of qualified types which should not be allowed to - 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"`. 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 84649ae8afa52b8..246a0d2a01d017f 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.SafeAwaitablesList: 'safe::awaitable; ::my::other::awaitable' \ +// RUN: }}" namespace std { @@ -135,14 +136,18 @@ ReturnObject scopedLockableTest() { absl::Mutex no_warning_5; } -struct [[clang::annotate("coro_raii_safe_suspend")]] raii_safe_suspend { +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 raii_safe_suspend{}; + co_await safe::awaitable{}; + using other = safe::awaitable; + co_await other{}; } void lambda() { >From fd455bf3bb32191e728f1961917d7eda5cdc4227 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 13:13:31 +0100 Subject: [PATCH 09/11] typo fix --- clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h index 3821e255a81c2c0..9399374518c24a3 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h @@ -43,8 +43,7 @@ class CoroutineHostileRAIICheck : public ClangTidyCheck { // 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 for which are considered safe to - // co_await. + // List of fully qualified awaitable types which are considered safe to co_await. std::vector<StringRef> SafeAwaitablesList; }; >From 73c512f312f7333722551f76c371601223eb507b Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 13:16:34 +0100 Subject: [PATCH 10/11] format --- clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h index 9399374518c24a3..b07351debc405ce 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h @@ -43,7 +43,8 @@ class CoroutineHostileRAIICheck : public ClangTidyCheck { // 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. + // List of fully qualified awaitable types which are considered safe to + // co_await. std::vector<StringRef> SafeAwaitablesList; }; >From 6a4d8d96ef3718b6c2d2187643007ac273a8c042 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 21 Nov 2023 13:27:43 +0100 Subject: [PATCH 11/11] refactor typeWithNameIn --- .../clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index 3a98db34b3a887d..c5623f40ad09058 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -59,6 +59,11 @@ AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>, return Node.getCommonExpr() && InnerMatcher.matches(*Node.getCommonExpr(), Finder, Builder); } + +auto typeWithNameIn(const std::vector<StringRef> &Names) { + return hasType( + hasCanonicalType(hasDeclaration(namedDecl(hasAnyName(Names))))); +} } // namespace CoroutineHostileRAIICheck::CoroutineHostileRAIICheck(StringRef Name, @@ -74,11 +79,8 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) { auto ScopedLockable = varDecl(hasType(hasCanonicalType(hasDeclaration( hasAttr(attr::Kind::ScopedLockable))))) .bind("scoped-lockable"); - auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration( - namedDecl(hasAnyName(RAIITypesList)))))) - .bind("raii"); - auto SafeSuspend = awaiatable(hasType(hasCanonicalType( - hasDeclaration(namedDecl(hasAnyName(SafeAwaitablesList)))))); + auto OtherRAII = varDecl(typeWithNameIn(RAIITypesList)).bind("raii"); + auto SafeSuspend = awaiatable(typeWithNameIn(SafeAwaitablesList)); Finder->addMatcher( expr(anyOf(coawaitExpr(unless(SafeSuspend)), coyieldExpr()), forEachPrevStmt( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits