[clang-tools-extra] [clang-tidy][performance-unnecessary-value-param] Avoid in coroutines (PR #140912)

[Dmitry Polukhin via cfe-commits]

https://github.com/dmpolukhin created https://github.com/llvm/llvm-project/pull/140912

Summary:
Replacing by-value parameters with passing by-reference is not safe for coroutines because the caller may be executed in parallel with the callee, which increases the chances of resulting in dangling references and hard-to-find crashes.

Test Plan: check-clang-tools

>From e3ce9bb18f165649b00db14b2282649315b28883 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.polukhin at gmail.com>
Date: Wed, 21 May 2025 07:54:02 -0700
Subject: [PATCH] [clang-tidy][performance-unnecessary-value-param] Avoid in
 coroutines

Summary:
Replacing by-value parameters with passing by-reference is not safe for coroutines
because the caller may be executed in parallel with the callee, which increases
the chances of resulting in dangling references and hard-to-find crashes.

Test Plan: check-clang-tools
---
 .../UnnecessaryValueParamCheck.cpp            |  1 +
 .../unnecessary-value-param-coroutine.cpp     | 81 +++++++++++++++++++
 2 files changed, 82 insertions(+)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp

diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index a877f9a7ee912..52d4c70a83f65 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -65,6 +65,7 @@ void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
           TK_AsIs,
           functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
                        unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
+                       unless(hasBody(coroutineBodyStmt())),
                        has(typeLoc(forEach(ExpensiveValueParamDecl))),
                        decl().bind("functionDecl"))),
       this);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp
new file mode 100644
index 0000000000000..f2fb477143578
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s performance-unnecessary-value-param %t -- -fix-errors
+
+namespace std {
+
+template <typename R, typename...> struct coroutine_traits {
+  using promise_type = typename R::promise_type;
+};
+
+template <typename Promise = void> struct coroutine_handle;
+
+template <> struct coroutine_handle<void> {
+  static coroutine_handle from_address(void *addr) noexcept {
+    coroutine_handle me;
+    me.ptr = addr;
+    return me;
+  }
+  void operator()() { resume(); }
+  void *address() const noexcept { return ptr; }
+  void resume() const {  }
+  void destroy() const { }
+  bool done() const { return true; }
+  coroutine_handle &operator=(decltype(nullptr)) {
+    ptr = nullptr;
+    return *this;
+  }
+  coroutine_handle(decltype(nullptr)) : ptr(nullptr) {}
+  coroutine_handle() : ptr(nullptr) {}
+  //  void reset() { ptr = nullptr; } // add to P0057?
+  explicit operator bool() const { return ptr; }
+
+protected:
+  void *ptr;
+};
+
+template <typename Promise> struct coroutine_handle : coroutine_handle<> {
+  using coroutine_handle<>::operator=;
+
+  static coroutine_handle from_address(void *addr) noexcept {
+    coroutine_handle me;
+    me.ptr = addr;
+    return me;
+  }
+
+  Promise &promise() const {
+    return *reinterpret_cast<Promise *>(
+        __builtin_coro_promise(ptr, alignof(Promise), false));
+  }
+  static coroutine_handle from_promise(Promise &promise) {
+    coroutine_handle p;
+    p.ptr = __builtin_coro_promise(&promise, alignof(Promise), true);
+    return p;
+  }
+};
+
+struct suspend_always {
+  bool await_ready() noexcept { return false; }
+  void await_suspend(std::coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+} // namespace std
+
+struct ReturnObject {
+    struct promise_type {
+        ReturnObject get_return_object() { return {}; }
+        std::suspend_always initial_suspend() { return {}; }
+        std::suspend_always final_suspend() noexcept { return {}; }
+        void unhandled_exception() {}
+        std::suspend_always yield_value(int value) { return {}; }
+    };
+};
+
+struct A {
+  A(const A&);
+};
+
+ReturnObject evaluateModels(const A timeMachineId) {
+// No change for non-coroutine function expected because it is not safe.
+// CHECK-FIXES: ReturnObject evaluateModels(const A timeMachineId) {
+  co_return;
+}