llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Utkarsh Saxena (usx95)
<details>
<summary>Changes</summary>
Certain `awaitable` types could be safe to `co_await` on even when we have
suspension-hostile RAII objects in scope.
This PR adds a way for users to mark such safe `awaitable` and silence false
positive warnings in `co_await` expressions involving such an `awaitable`.
``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.
---
Full diff: https://github.com/llvm/llvm-project/pull/72954.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
(+21-5)
- (modified)
clang-tools-extra/docs/clang-tidy/checks/misc/coroutine-hostile-raii.rst
(+40-4)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp
(+10)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
index e820cd39d83d21b..ee7565d5c714d4d 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, 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,
@@ -68,11 +81,14 @@ void
CoroutineHostileRAIICheck::registerMatchers(MatchFinder *Finder) {
auto OtherRAII = varDecl(hasType(hasCanonicalType(hasDeclaration(
namedDecl(hasAnyName(RAIITypesList))))))
.bind("raii");
- Finder->addMatcher(expr(anyOf(coawaitExpr(), coyieldExpr()),
- forEachPrevStmt(declStmt(forEach(
- varDecl(anyOf(ScopedLockable, OtherRAII))))))
- .bind("suspension"),
- this);
+ auto SafeSuspend =
+ awaiatable(hasType(hasCanonicalType(hasDeclaration(isRAIISafe()))));
+ 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) {
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..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
@@ -29,15 +29,51 @@ 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();
}
+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 {
+ 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{};
+ }
+
+ auto wait() { return safe_awaitable{}; }
+
+ task coro() {
+ const std::lock_guard l(&mu_); // No warning.
+ co_await safe_awaitable{};
+ co_await wait();
+ }
Options
-------
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..84649ae8afa52b8 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
@@ -135,6 +135,16 @@ 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{};
+}
+
void lambda() {
absl::Mutex no_warning;
auto lambda = []() -> ReturnObject {
``````````
</details>
https://github.com/llvm/llvm-project/pull/72954
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
