[clang-tools-extra] 26d082d - [clang-tidy][performance-unnecessary-value-param] Avoid in coroutines (#140912)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 17 01:47:19 PDT 2025
Author: Dmitry Polukhin
Date: 2025-06-17T09:47:15+01:00
New Revision: 26d082d330e4d8d1fc3194b4b87ede9332a297f5
URL: https://github.com/llvm/llvm-project/commit/26d082d330e4d8d1fc3194b4b87ede9332a297f5
DIFF: https://github.com/llvm/llvm-project/commit/26d082d330e4d8d1fc3194b4b87ede9332a297f5.diff
LOG: [clang-tidy][performance-unnecessary-value-param] Avoid in coroutines (#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. See for the reference
[cppcoreguidelines-avoid-reference-coroutine-parameters](https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.html).
Test Plan: check-clang-tools
Added:
clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp
Modified:
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index a877f9a7ee912..d89c3a69fc841 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/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", ""))),
+ IgnoreCoroutines(Options.get("IgnoreCoroutines", true)) {}
void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
const auto ExpensiveValueParamDecl = parmVarDecl(
@@ -61,12 +62,14 @@ void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
matchers::matchesAnyListedName(AllowedTypes))))))),
decl().bind("param"));
Finder->addMatcher(
- traverse(
- TK_AsIs,
- functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
- unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
- has(typeLoc(forEach(ExpensiveValueParamDecl))),
- decl().bind("functionDecl"))),
+ traverse(TK_AsIs,
+ functionDecl(
+ hasBody(IgnoreCoroutines ? stmt(unless(coroutineBodyStmt()))
+ : stmt()),
+ isDefinition(), unless(isImplicit()),
+ unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
+ has(typeLoc(forEach(ExpensiveValueParamDecl))),
+ decl().bind("functionDecl"))),
this);
}
@@ -123,6 +126,7 @@ void UnnecessaryValueParamCheck::storeOptions(
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
Options.store(Opts, "AllowedTypes",
utils::options::serializeStringList(AllowedTypes));
+ Options.store(Opts, "IgnoreCoroutines", IgnoreCoroutines);
}
void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h
index 8bfd814d16357..b52043416e769 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ b/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 IgnoreCoroutines;
};
} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 19ccd1790e757..3c1ca2f929044 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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 `IgnoreCoroutines` with the default value `true` 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
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
index dc86530b95f13..cd25d7d94d99b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
+++ b/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,10 @@ 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:: IgnoreCoroutines
+
+ A boolean specifying whether the check should suggest passing parameters by
+ reference in coroutines. Passing parameters by reference in coroutines may
+ not be safe, please see :doc:`cppcoreguidelines-avoid-reference-coroutine-parameters <../cppcoreguidelines/avoid-reference-coroutine-parameters>`
+ for more information. Default is `true`.
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..0a84dc4676470
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-coroutine.cpp
@@ -0,0 +1,65 @@
+// 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.IgnoreCoroutines: true}}' -fix-errors
+// RUN: %check_clang_tidy -check-suffix=ALLOWED -std=c++20-or-later %s performance-unnecessary-value-param %t -- \
+// RUN: -config='{CheckOptions: {performance-unnecessary-value-param.IgnoreCoroutines: false}}' -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 {}; }
+ ReturnObject return_void() { 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 foo_coroutine(const A a) {
+// CHECK-MESSAGES-ALLOWED: [[@LINE-1]]:36: warning: the const qualified parameter 'a'
+// CHECK-FIXES: ReturnObject foo_coroutine(const A a) {
+ co_return;
+}
+
+ReturnObject foo_not_coroutine(const A a) {
+// CHECK-MESSAGES: [[@LINE-1]]:40: warning: the const qualified parameter 'a'
+// CHECK-MESSAGES-ALLOWED: [[@LINE-2]]:40: warning: the const qualified parameter 'a'
+ return ReturnObject{};
+}
More information about the cfe-commits
mailing list