llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Helmut Januschka (hjanuschka)
<details>
<summary>Changes</summary>
Add new clang-tidy check that finds functions returning `void` with a single
non-const reference output parameter, suggesting they return the value directly
instead.
Returning values instead of using output parameters is generally preferred in
modern C++ because it is clearer, works better with `auto` and structured
bindings, and enables copy/move elision.
```cpp
// Before
void getResult(int &Out) {
Out = compute();
}
// After
int getResult() {
return compute();
}
```
Cases intentionally **not** flagged:
- Non-void return type
- Zero or more than one non-const reference parameter
- No assignment to the output parameter
- Abstract or array type output parameters
- Virtual methods
- Unnamed output parameters
No fix-its are provided since changing the return type requires updating all
call sites, which may span translation units.
---
Full diff: https://github.com/llvm/llvm-project/pull/182081.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
- (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
(+3)
- (added) clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.cpp (+167)
- (added) clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.h (+34)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added)
clang-tools-extra/docs/clang-tidy/checks/modernize/use-return-value.rst (+39)
- (added)
clang-tools-extra/test/clang-tidy/checkers/modernize/use-return-value.cpp (+73)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index cc4cc7a02b594..312ba733d59ca 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -45,6 +45,7 @@ add_clang_library(clangTidyModernizeModule STATIC
UseNullptrCheck.cpp
UseOverrideCheck.cpp
UseRangesCheck.cpp
+ UseReturnValueCheck.cpp
UseScopedLockCheck.cpp
UseStartsEndsWithCheck.cpp
UseStdFormatCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fcb860d8c5298..40ab9e593f6d5 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -45,6 +45,7 @@
#include "UseNullptrCheck.h"
#include "UseOverrideCheck.h"
#include "UseRangesCheck.h"
+#include "UseReturnValueCheck.h"
#include "UseScopedLockCheck.h"
#include "UseStartsEndsWithCheck.h"
#include "UseStdFormatCheck.h"
@@ -92,6 +93,8 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<UseIntegerSignComparisonCheck>(
"modernize-use-integer-sign-comparison");
CheckFactories.registerCheck<UseRangesCheck>("modernize-use-ranges");
+ CheckFactories.registerCheck<UseReturnValueCheck>(
+ "modernize-use-return-value");
CheckFactories.registerCheck<UseScopedLockCheck>(
"modernize-use-scoped-lock");
CheckFactories.registerCheck<UseStartsEndsWithCheck>(
diff --git a/clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.cpp
new file mode 100644
index 0000000000000..f9472c77d21ff
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.cpp
@@ -0,0 +1,167 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseReturnValueCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+
+/// Find the single non-const lvalue reference parameter in a function that
+/// could serve as an output parameter. Returns nullptr if there are zero or
+/// more than one such parameters, or if the parameter type is not suitable.
+static const ParmVarDecl *findSingleOutParam(const FunctionDecl *Func) {
+ const ParmVarDecl *Candidate = nullptr;
+ for (const auto *Param : Func->parameters()) {
+ const QualType T = Param->getType();
+ if (!T->isLValueReferenceType())
+ continue;
+ const QualType Pointee = T->getPointeeType();
+ // Skip const references -- those are input parameters.
+ if (Pointee.isConstQualified())
+ continue;
+ // Skip references to non-object types.
+ if (Pointee->isFunctionType() || Pointee->isVoidType())
+ continue;
+ // More than one non-const ref param -- ambiguous.
+ if (Candidate)
+ return nullptr;
+ Candidate = Param;
+ }
+ return Candidate;
+}
+
+/// Visitor that checks whether a parameter is only written to (never read
+/// before being fully assigned). A parameter is an "output-only" parameter if
+/// every use of it in the function body is an assignment target or passed to
+/// a function that takes a non-const reference (i.e., it is used purely for
+/// output).
+class OutParamVisitor : public RecursiveASTVisitor<OutParamVisitor> {
+public:
+ explicit OutParamVisitor(const ParmVarDecl *Param) : Param(Param) {}
+
+ bool isOutputOnly() const { return HasWrite && !HasRead; }
+
+ bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
+ if (DRE->getDecl() != Param)
+ return true;
+
+ // Check how this reference is used by looking at the parent.
+ // We conservatively mark any use we cannot classify as a read.
+ HasWrite = true;
+ HasRead = true; // Conservative default; refined by parent checks.
+ return true;
+ }
+
+ // Override to check parent context of DeclRefExprs.
+ bool TraverseStmt(Stmt *S) {
+ if (!S)
+ return true;
+ return RecursiveASTVisitor::TraverseStmt(S);
+ }
+
+ /// Check if the param is used as an assignment LHS:
+ /// param = expr; (direct assignment)
+ /// param.field = expr; (member assignment)
+ bool VisitBinaryOperator(const BinaryOperator *BO) {
+ if (BO->getOpcode() != BO_Assign)
+ return true;
+ const Expr *LHS = BO->getLHS()->IgnoreImplicit();
+ // Direct assignment: param = expr.
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(LHS))
+ if (DRE->getDecl() == Param)
+ FoundAssignment = true;
+ // Member assignment: param.field = expr.
+ if (const auto *ME = dyn_cast<MemberExpr>(LHS))
+ if (const auto *DRE =
+ dyn_cast<DeclRefExpr>(ME->getBase()->IgnoreImplicit()))
+ if (DRE->getDecl() == Param)
+ FoundAssignment = true;
+ return true;
+ }
+
+ bool hasAssignment() const { return FoundAssignment; }
+
+private:
+ const ParmVarDecl *Param;
+ bool HasWrite = false;
+ bool HasRead = false;
+ bool FoundAssignment = false;
+};
+
+} // namespace
+
+void UseReturnValueCheck::registerMatchers(MatchFinder *Finder) {
+ // Match non-template function definitions returning void.
+ Finder->addMatcher(
+ functionDecl(
+ isDefinition(),
+ unless(isImplicit()),
+ unless(isDeleted()),
+ returns(voidType()),
+ unless(hasParent(functionTemplateDecl())),
+ // At least one non-const ref parameter.
+ hasAnyParameter(parmVarDecl(hasType(lValueReferenceType(
+ pointee(unless(isConstQualified())))))))
+ .bind("func"),
+ this);
+}
+
+void UseReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ if (!Func || !Func->hasBody())
+ return;
+
+ // Skip system headers.
+ if (Result.SourceManager->isInSystemHeader(Func->getLocation()))
+ return;
+
+ // Skip virtual methods (changing return type breaks polymorphism).
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(Func))
+ if (MD->isVirtual())
+ return;
+
+ // Skip main.
+ if (Func->isMain())
+ return;
+
+ // Find the single output parameter.
+ const ParmVarDecl *OutParam = findSingleOutParam(Func);
+ if (!OutParam || !OutParam->getIdentifier())
+ return;
+
+ // The output parameter type must be copyable/movable (not an abstract
+ // class, not an array, etc.).
+ const QualType ParamType = OutParam->getType()->getPointeeType();
+ if (ParamType->isArrayType() || ParamType->isIncompleteType())
+ return;
+ if (const auto *RD = ParamType->getAsCXXRecordDecl())
+ if (RD->isAbstract())
+ return;
+
+ // Verify the parameter is actually assigned to in the body.
+ OutParamVisitor Visitor(OutParam);
+ Visitor.TraverseStmt(const_cast<Stmt *>(Func->getBody()));
+
+ if (!Visitor.hasAssignment())
+ return;
+
+ diag(Func->getLocation(),
+ "function '%0' has output parameter '%1'; consider returning "
+ "the value directly instead")
+ << Func->getName() << OutParam->getName();
+ diag(OutParam->getLocation(),
+ "output parameter declared here", DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.h
b/clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.h
new file mode 100644
index 0000000000000..4cd636c82445f
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseReturnValueCheck.h
@@ -0,0 +1,34 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USERETURNVALUECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USERETURNVALUECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Finds functions that return void and have a single non-const reference
+/// output parameter, suggesting they return the value directly instead.
+///
+/// For the user-facing documentation see:
+///
https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-return-value.html
+class UseReturnValueCheck : public ClangTidyCheck {
+public:
+ UseReturnValueCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USERETURNVALUECHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 68bab88146241..712d0dd18bb59 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -121,6 +121,12 @@ New checks
``llvm::to_vector(llvm::make_filter_range(...))`` that can be replaced with
``llvm::map_to_vector`` and ``llvm::filter_to_vector``.
+- New :doc:`modernize-use-return-value
+ <clang-tidy/checks/modernize/use-return-value>` check.
+
+ Finds ``void`` functions with a single non-const reference output
+ parameter, suggesting they return the value directly instead.
+
- New :doc:`modernize-use-string-view
<clang-tidy/checks/modernize/use-string-view>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c475870ed7b31..c52be3c225bff 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -325,6 +325,7 @@ Clang-Tidy Checks
:doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes"
:doc:`modernize-use-override <modernize/use-override>`, "Yes"
:doc:`modernize-use-ranges <modernize/use-ranges>`, "Yes"
+ :doc:`modernize-use-return-value <modernize/use-return-value>`,
:doc:`modernize-use-scoped-lock <modernize/use-scoped-lock>`, "Yes"
:doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`,
"Yes"
:doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes"
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-return-value.rst
b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-return-value.rst
new file mode 100644
index 0000000000000..281647f4d6f8c
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-return-value.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - modernize-use-return-value
+
+modernize-use-return-value
+==========================
+
+Finds functions that return ``void`` and have a single non-const
+reference output parameter, suggesting they return the value
+directly instead.
+
+Returning values instead of using output parameters is generally
+preferred in modern C++ because it is clearer, works better with
+``auto`` and structured bindings, and enables copy/move elision.
+
+.. code-block:: c++
+
+ // Before
+ void getResult(int &Out) {
+ Out = compute();
+ }
+
+ // After
+ int getResult() {
+ return compute();
+ }
+
+The check will not flag a function if it:
+
+- returns non-void,
+- has zero or more than one non-const reference parameter,
+- never assigns to the output parameter,
+- has an output parameter of abstract or array type,
+- is a virtual method, or
+- has an unnamed output parameter.
+
+.. note::
+
+ This check does not provide fix-its because changing the
+ return type requires updating all call sites, which may
+ span multiple translation units.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-return-value.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-return-value.cpp
new file mode 100644
index 0000000000000..c748606e7deab
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-return-value.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s modernize-use-return-value %t
+
+struct Widget {
+ int X;
+};
+
+// Positive: void function with single non-const ref output param.
+void getInt(int &Out) {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'getInt' has output
parameter 'Out'
+ Out = 42;
+}
+
+// Positive: struct output parameter.
+void getWidget(Widget &Out) {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'getWidget' has output
parameter 'Out'
+ Out.X = 10;
+}
+
+// Positive: mixed const and non-const ref params (one non-const).
+void transform(const int &In, int &Out) {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'transform' has output
parameter 'Out'
+ Out = In * 2;
+}
+
+// Positive: non-const ref + value params.
+void compute(int A, int B, int &Result) {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'compute' has output
parameter 'Result'
+ Result = A + B;
+}
+
+// Negative: returns non-void.
+int getX(int &Out) {
+ Out = 42;
+ return 0;
+}
+
+// Negative: const ref parameter (input, not output).
+void readOnly(const int &X) {
+}
+
+// Negative: multiple non-const ref params (ambiguous outparam).
+void swap(int &A, int &B) {
+ int T = A;
+ A = B;
+ B = T;
+}
+
+// Negative: no assignment to the parameter.
+void noWrite(int &X) {
+ int Y = X + 1;
+}
+
+// Negative: virtual method.
+struct Base {
+ virtual void vmethod(int &Out);
+};
+
+// Negative: abstract output type.
+struct Abstract {
+ virtual void foo() = 0;
+};
+void getAbstract(Abstract &Out) {
+ // Not flagged -- Abstract cannot be returned by value.
+}
+
+// Negative: array type.
+void getArray(int (&Out)[10]) {
+ Out[0] = 1;
+}
+
+// Negative: unnamed parameter.
+void unnamed(int &) {
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/182081
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
