[clang-tidy][performance-unnecessary-value-param] Avoid in coroutines

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);

clang-tools-extra/docs/ReleaseNotes.rst

@@ -183,7 +183,7 @@ Changes in existing checks
   explicit casting of built-in types within member list initialization.
 
 - Improved :doc:`modernize-use-ranges
-  <clang-tidy/checks/modernize/use-ranges>` check by updating suppress 
+  <clang-tidy/checks/modernize/use-ranges>` check by updating suppress
   warnings logic for ``nullptr`` in ``std::find``.
 
 - Improved :doc:`modernize-use-starts-ends-with
@@ -203,6 +203,8 @@ Changes in existing checks
   <clang-tidy/checks/performance/unnecessary-value-param>` check performance by
   tolerating fix-it breaking compilation when functions is used as pointers
   to avoid matching usage of functions within the current compilation unit.
+  Also suppressed this check for coroutine functions because it may not be safe
+  and suggested fixes may result in hard-to-find bugs and crashes.
 
 Removed checks
 ^^^^^^^^^^^^^^

clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst

@@ -56,7 +56,11 @@ Will become:
 
 Because the fix-it needs to change the signature of the function, it may break
 builds if the function is used in multiple translation units or some codes
-depends on funcion signatures.
+depends on function signatures.
+
+Note: This check does not suggest passing parameters by reference in coroutines
+because, after a coroutine suspend point, references could be dangling and no
+longer valid, so suggested changes may result in hard-to-find bugs and crashes.
 
 Options
 -------

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp

@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s performance-unnecessary-value-param %t -- -fix-errors
+
+namespace std {
+
+template <class Ret, typename... T> struct coroutine_traits {
+  using promise_type = typename Ret::promise_type;
+};
+
+template <class Promise = void> struct coroutine_handle {
+  static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(Promise &promise);
+  constexpr void *address() const noexcept;
+};
+
+template <> struct coroutine_handle<void> {
+  template <class PromiseType>
+  coroutine_handle(coroutine_handle<PromiseType>) noexcept;
+  static coroutine_handle from_address(void *);
+  constexpr void *address() const noexcept;
+};
+
+struct suspend_always {
+  bool await_ready() noexcept { return false; }
+  void await_suspend(coroutine_handle<>) noexcept {}
+  void await_resume() noexcept {}
+};
+
+struct suspend_never {
+  bool await_ready() noexcept { return true; }
+  void await_suspend(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;
+}