[flang-commits] [clang-tools-extra] [llvm] [lld] [mlir] [clang] [flang] [clangtidy] Allow safe suspensions in coroutine-hostile-raii check (PR #72954)
Utkarsh Saxena via flang-commits
flang-commits at lists.llvm.org
Mon Nov 27 07:20:30 PST 2023
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 <usx at google.com>
Date: Tue, 21 Nov 2023 06:51:48 +0100
Subject: [PATCH 01/14] [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 <usx at google.com>
Date: Tue, 21 Nov 2023 07:03:16 +0100
Subject: [PATCH 02/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 07:06:51 +0100
Subject: [PATCH 03/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 07:40:13 +0100
Subject: [PATCH 04/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 08:02:46 +0100
Subject: [PATCH 05/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 09:00:39 +0100
Subject: [PATCH 06/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 09:26:15 +0100
Subject: [PATCH 07/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 13:09:31 +0100
Subject: [PATCH 08/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 13:13:31 +0100
Subject: [PATCH 09/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 13:16:34 +0100
Subject: [PATCH 10/14] 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 <usx at google.com>
Date: Tue, 21 Nov 2023 13:27:43 +0100
Subject: [PATCH 11/14] 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(
>From fd7aaf63d039bc82abeb9d317dfaed62001cd222 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 21 Nov 2023 13:29:47 +0100
Subject: [PATCH 12/14] typo fix
---
.../docs/clang-tidy/checks/misc/coroutine-hostile-raii.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 4e99ebc7b5c4cb7..5dd951615100797 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
@@ -62,7 +62,7 @@ Options
.. code-block:: c++
- // Cosnider option SafeAwaitablesList = "safe_awaitable"
+ // Consider option SafeAwaitablesList = "safe_awaitable"
struct safe_awaitable {
bool await_ready() noexcept { return false; }
void await_suspend(std::coroutine_handle<>) noexcept {}
>From 411e0ab1b624a1a34e9fa56d5c65730d59cfc76c Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 21 Nov 2023 16:05:07 +0100
Subject: [PATCH 13/14] address comments
---
.../clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 15 ++++++++-------
.../clang-tidy/misc/CoroutineHostileRAIICheck.h | 2 +-
.../checks/misc/coroutine-hostile-raii.rst | 4 ++--
.../checkers/misc/coroutine-hostile-raii.cpp | 2 +-
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
index c5623f40ad09058..6c8dd671539a813 100644
--- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
@@ -56,8 +56,9 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>,
// 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);
+ if (Expr *E = Node.getCommonExpr())
+ return InnerMatcher.matches(*E, Finder, Builder);
+ return false;
}
auto typeWithNameIn(const std::vector<StringRef> &Names) {
@@ -71,8 +72,8 @@ CoroutineHostileRAIICheck::CoroutineHostileRAIICheck(StringRef Name,
: ClangTidyCheck(Name, Context),
RAIITypesList(utils::options::parseStringList(
Options.get("RAIITypesList", "std::lock_guard;std::scoped_lock"))),
- SafeAwaitablesList(utils::options::parseStringList(
- Options.get("SafeAwaitablesList", ""))) {}
+ AllowedAwaitablesList(utils::options::parseStringList(
+ Options.get("AllowedAwaitablesList", ""))) {}
void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) {
// A suspension happens with co_await or co_yield.
@@ -80,9 +81,9 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) {
hasAttr(attr::Kind::ScopedLockable)))))
.bind("scoped-lockable");
auto OtherRAII = varDecl(typeWithNameIn(RAIITypesList)).bind("raii");
- auto SafeSuspend = awaiatable(typeWithNameIn(SafeAwaitablesList));
+ auto AllowedSuspend = awaiatable(typeWithNameIn(AllowedAwaitablesList));
Finder->addMatcher(
- expr(anyOf(coawaitExpr(unless(SafeSuspend)), coyieldExpr()),
+ expr(anyOf(coawaitExpr(unless(AllowedSuspend)), coyieldExpr()),
forEachPrevStmt(
declStmt(forEach(varDecl(anyOf(ScopedLockable, OtherRAII))))))
.bind("suspension"),
@@ -109,6 +110,6 @@ void CoroutineHostileRAIICheck::storeOptions(
Options.store(Opts, "RAIITypesList",
utils::options::serializeStringList(RAIITypesList));
Options.store(Opts, "SafeAwaitableList",
- utils::options::serializeStringList(SafeAwaitablesList));
+ 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 b07351debc405ce..be925097692a485 100644
--- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h
+++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.h
@@ -45,7 +45,7 @@ class CoroutineHostileRAIICheck : public ClangTidyCheck {
std::vector<StringRef> RAIITypesList;
// List of fully qualified awaitable types which are considered safe to
// co_await.
- std::vector<StringRef> SafeAwaitablesList;
+ std::vector<StringRef> 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 5dd951615100797..a39c1853b313c49 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
@@ -48,7 +48,7 @@ Options
Eg: ``my::lockable; a::b;::my::other::lockable;``
The default value of this option is `"std::lock_guard;std::scoped_lock"`.
-.. option:: SafeAwaitablesList
+.. option:: AllowedAwaitablesList
A semicolon-separated list of qualified types of awaitables types which can
be safely awaited while having hostile RAII objects in scope.
@@ -62,7 +62,7 @@ Options
.. code-block:: c++
- // Consider option SafeAwaitablesList = "safe_awaitable"
+ // Consider option AllowedAwaitablesList = "safe_awaitable"
struct safe_awaitable {
bool await_ready() noexcept { return false; }
void await_suspend(std::coroutine_handle<>) noexcept {}
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 246a0d2a01d017f..55a7e4b8f2954aa 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,7 @@
// RUN: %check_clang_tidy -std=c++20 %s misc-coroutine-hostile-raii %t \
// 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: misc-coroutine-hostile-raii.AllowedAwaitablesList: 'safe::awaitable; ::my::other::awaitable' \
// RUN: }}"
namespace std {
>From b33e46ae67bbd73aeba72514ab27794c0b472466 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 27 Nov 2023 16:20:08 +0100
Subject: [PATCH 14/14] fix typo
---
.../clang-tidy/misc/CoroutineHostileRAIICheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
index 6c8dd671539a813..a0e8700b0522bc5 100644
--- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
@@ -54,7 +54,7 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>,
}
// Matches the expression awaited by the `co_await`.
-AST_MATCHER_P(CoawaitExpr, awaiatable, ast_matchers::internal::Matcher<Expr>,
+AST_MATCHER_P(CoawaitExpr, awaitable, ast_matchers::internal::Matcher<Expr>,
InnerMatcher) {
if (Expr *E = Node.getCommonExpr())
return InnerMatcher.matches(*E, Finder, Builder);
@@ -81,7 +81,7 @@ void CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) {
hasAttr(attr::Kind::ScopedLockable)))))
.bind("scoped-lockable");
auto OtherRAII = varDecl(typeWithNameIn(RAIITypesList)).bind("raii");
- auto AllowedSuspend = awaiatable(typeWithNameIn(AllowedAwaitablesList));
+ auto AllowedSuspend = awaitable(typeWithNameIn(AllowedAwaitablesList));
Finder->addMatcher(
expr(anyOf(coawaitExpr(unless(AllowedSuspend)), coyieldExpr()),
forEachPrevStmt(
More information about the flang-commits
mailing list