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

[Utkarsh Saxena via flang-commits]

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/3] [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/3] 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/3] 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) {