[flang] [mlir] [clang] [llvm] [lld] [clang-tools-extra] [clangtidy] Allow safe suspensions in coroutine-hostile-raii check (PR #72954)
Utkarsh Saxena via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 20 22:46:29 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 1/4] [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 2/4] 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 3/4] 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 4/4] 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{};
More information about the llvm-commits
mailing list