Added option IsAllowedInCoroutines

Author: dmpolukhin

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(
@@ -65,7 +66,6 @@ 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);
@@ -74,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);
 
@@ -124,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,8 +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.
-  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.
+  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

@@ -58,10 +58,6 @@ 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 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
 -------
 
@@ -78,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

@@ -1,4 +1,8 @@
 // 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 {