[flang-commits] [flang] [mlir] [clang] [llvm] [lld] [clang-tools-extra] [clangtidy] Allow safe suspensions in coroutine-hostile-raii check (PR #72954)

Utkarsh Saxena via flang-commits flang-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 flang-commits mailing list