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

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

@@ -50,7 +50,8 @@ UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
                                         utils::IncludeSorter::IS_LLVM),
                areDiagsSelfContained()),
       AllowedTypes(
-          utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
+          utils::options::parseStringList(Options.get("AllowedTypes", ""))),
+      IsAllowedInCoroutines(Options.get("IsAllowedInCoroutines", false)) {}
 
 void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
   const auto ExpensiveValueParamDecl = parmVarDecl(
@@ -73,6 +74,10 @@ void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
 void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
   const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+  if (!IsAllowedInCoroutines) {
+    if (auto Body = Function->getBody(); Body && isa<CoroutineBodyStmt>(Body))
+      return;
+  }
 
   TraversalKindScope RAII(*Result.Context, TK_AsIs);
 
@@ -123,6 +128,7 @@ void UnnecessaryValueParamCheck::storeOptions(
   Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "AllowedTypes",
                 utils::options::serializeStringList(AllowedTypes));
+  Options.store(Opts, "IsAllowedInCoroutines", IsAllowedInCoroutines);
 }
 
 void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h

@@ -46,6 +46,7 @@ class UnnecessaryValueParamCheck : public ClangTidyCheck {
   ExprMutationAnalyzer::Memoized MutationAnalyzerCache;
   utils::IncludeInserter Inserter;
   const std::vector<StringRef> AllowedTypes;
+  bool IsAllowedInCoroutines;
 };
 
 } // namespace clang::tidy::performance

clang-tools-extra/docs/ReleaseNotes.rst

@@ -265,6 +265,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.
+  Added an option `IsAllowedInCoroutines` with the default value ``false`` to
+  suppress this check for coroutines where passing by reference may be unsafe.
 
 - Improved :doc:`readability-convert-member-functions-to-static
   <clang-tidy/checks/readability/convert-member-functions-to-static>` check by

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

@@ -56,7 +56,7 @@ 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.
 
 Options
 -------
@@ -74,3 +74,9 @@ Options
    default is empty. If a name in the list contains the sequence `::`, it is
    matched against the qualified type name (i.e. ``namespace::Type``),
    otherwise it is matched against only the type name (i.e. ``Type``).
+
+.. option:: IsAllowedInCoroutines
+
+   A boolean specifying whether the check should suggest passing parameters by
+   reference in coroutines. The default is `false`. Passing parameters by reference
+   in coroutines may not be safe.

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

@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s performance-unnecessary-value-param %t -- -fix-errors
+// RUN: %check_clang_tidy -std=c++20-or-later %s performance-unnecessary-value-param %t -- \
+// RUN:   -config='{CheckOptions: {performance-unnecessary-value-param.IsAllowedInCoroutines: false}}' -fix-errors
+// RUN: not %check_clang_tidy -std=c++20-or-later %s performance-unnecessary-value-param %t -- \
+// RUN:   -config='{CheckOptions: {performance-unnecessary-value-param.IsAllowedInCoroutines: true}}' -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;
+}