https://github.com/SimplyDanny updated https://github.com/llvm/llvm-project/pull/76249
From 0daffd13160bc10e15e36327e596f8cabb96706e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 22 Dec 2023 17:09:59 +0100 Subject: [PATCH 1/9] [clang-tidy] Add check readability-return-expression-in-void-function --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 ++ .../ReturnExpressionInVoidFunctionCheck.cpp | 31 +++++++++++++++++++ .../ReturnExpressionInVoidFunctionCheck.h | 30 ++++++++++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 7 +++++ .../docs/clang-tidy/checks/list.rst | 1 + .../return-expression-in-void-function.rst | 30 ++++++++++++++++++ .../return-expression-in-void-function.cpp | 23 ++++++++++++++ 8 files changed, 126 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a4617..7d0399ed901267 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -42,6 +42,7 @@ add_clang_library(clangTidyReadabilityModule RedundantStringCStrCheck.cpp RedundantStringInitCheck.cpp ReferenceToConstructedTemporaryCheck.cpp + ReturnExpressionInVoidFunctionCheck.cpp SimplifyBooleanExprCheck.cpp SimplifySubscriptExprCheck.cpp StaticAccessedThroughInstanceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e641432060..2a9134071b9a72 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -45,6 +45,7 @@ #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" #include "ReferenceToConstructedTemporaryCheck.h" +#include "ReturnExpressionInVoidFunctionCheck.h" #include "SimplifyBooleanExprCheck.h" #include "SimplifySubscriptExprCheck.h" #include "StaticAccessedThroughInstanceCheck.h" @@ -119,6 +120,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-redundant-preprocessor"); CheckFactories.registerCheck<ReferenceToConstructedTemporaryCheck>( "readability-reference-to-constructed-temporary"); + CheckFactories.registerCheck<ReturnExpressionInVoidFunctionCheck>( + "readability-return-expression-in-void-function"); CheckFactories.registerCheck<SimplifySubscriptExprCheck>( "readability-simplify-subscript-expr"); CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>( diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp new file mode 100644 index 00000000000000..2308d0782ae1c3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp @@ -0,0 +1,31 @@ +//===--- ReturnExpressionInVoidFunctionCheck.cpp - clang-tidy -------------===// +// +// 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 "ReturnExpressionInVoidFunctionCheck.h" +#include "clang/AST/Stmt.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +void ReturnExpressionInVoidFunctionCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher( + returnStmt(hasReturnValue(hasType(voidType()))).bind("void_return"), + this); +} + +void ReturnExpressionInVoidFunctionCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); + diag(VoidReturn->getBeginLoc(), "return statements should not return void"); +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h new file mode 100644 index 00000000000000..e28f9cebd2620e --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h @@ -0,0 +1,30 @@ +//===--- ReturnExpressionInVoidFunctionCheck.h - clang-tidy -----*- C++ -*-===// +// +// 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_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Looks for statements returning expressions of type `void`. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/return-expression-in-void-function.html +class ReturnExpressionInVoidFunctionCheck : public ClangTidyCheck { +public: + ReturnExpressionInVoidFunctionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d91748e4cef18..20896df0d340aa 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -218,6 +218,13 @@ New checks Detects C++ code where a reference variable is used to extend the lifetime of a temporary object that has just been constructed. +- New :doc:`readability-return-expression-in-void-function + <clang-tidy/checks/readability/return-expression-in-void-function>` check. + + Complains about statements returning expressions of type ``void``. It can be + confusing if a function returns an expression even though its return type is + ``void``. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 31f0e090db1d7d..f1282b4a6b766e 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -371,6 +371,7 @@ Clang-Tidy Checks :doc:`readability-redundant-string-cstr <readability/redundant-string-cstr>`, "Yes" :doc:`readability-redundant-string-init <readability/redundant-string-init>`, "Yes" :doc:`readability-reference-to-constructed-temporary <readability/reference-to-constructed-temporary>`, + :doc:`readability-return-expression-in-void-function <readability/return-expression-in-void-function>`, "Yes" :doc:`readability-simplify-boolean-expr <readability/simplify-boolean-expr>`, "Yes" :doc:`readability-simplify-subscript-expr <readability/simplify-subscript-expr>`, "Yes" :doc:`readability-static-accessed-through-instance <readability/static-accessed-through-instance>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst new file mode 100644 index 00000000000000..dd07b95550e6f8 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst @@ -0,0 +1,30 @@ +.. title:: clang-tidy - readability-return-expression-in-void-function + +readability-return-expression-in-void-function +============================================== + +Complains about statements returning expressions of type ``void``. It can be +confusing if a function returns an expression even though its return type is +``void``. + +Example: + +.. code-block:: + + void g(); + void f() { + // ... + return g(); + } + +In a long function body, the ``return`` statements suggests that the function +returns a value. However, ``return g();`` is combination of two statements that +should be written as + +.. code-block:: + + g(); + return; + +to make clear that ``g()`` is called and immediately afterwards the function +returns (nothing). diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp new file mode 100644 index 00000000000000..05e2f454abcd35 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp @@ -0,0 +1,23 @@ +// RUN: %check_clang_tidy %s readability-return-expression-in-void-function %t + +void f1(); + +void f2() { + return f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function] +} + +void f3(bool b) { + if (b) return f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statements should not return void [readability-return-expression-in-void-function] + return f2(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function] +} + +template<class T> +T f4() {} + +void f5() { + return f4<void>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function] +} From 6c23b9e929d406f79c1b2c1fee7ea2b282b544ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 22 Dec 2023 18:14:07 +0100 Subject: [PATCH 2/9] Clarify check message --- .../readability/ReturnExpressionInVoidFunctionCheck.cpp | 3 ++- .../readability/return-expression-in-void-function.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp index 2308d0782ae1c3..6517a4f8323c6b 100644 --- a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp @@ -25,7 +25,8 @@ void ReturnExpressionInVoidFunctionCheck::registerMatchers( void ReturnExpressionInVoidFunctionCheck::check( const MatchFinder::MatchResult &Result) { const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); - diag(VoidReturn->getBeginLoc(), "return statements should not return void"); + diag(VoidReturn->getBeginLoc(), + "return statement in void function should not return a value"); } } // namespace clang::tidy::readability diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp index 05e2f454abcd35..eda7bcd36c2ea5 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp @@ -4,14 +4,14 @@ void f1(); void f2() { return f1(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] } void f3(bool b) { if (b) return f1(); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statements should not return void [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] return f2(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] } template<class T> @@ -19,5 +19,5 @@ T f4() {} void f5() { return f4<void>(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statements should not return void [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] } From f4f9378f2b09f8342236532f1bf1dd76bac009b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 22 Dec 2023 18:14:24 +0100 Subject: [PATCH 3/9] Specify traversal kind --- .../readability/ReturnExpressionInVoidFunctionCheck.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h index e28f9cebd2620e..39c9553d602728 100644 --- a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h @@ -23,6 +23,9 @@ class ReturnExpressionInVoidFunctionCheck : public ClangTidyCheck { : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } }; } // namespace clang::tidy::readability From 7217d5623c5cbf57fb8672056b8274ad7118c882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 22 Dec 2023 18:15:41 +0100 Subject: [PATCH 4/9] Do not claim that check provides fixes --- clang-tools-extra/docs/clang-tidy/checks/list.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index f1282b4a6b766e..b32660aac7e6d0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -371,7 +371,7 @@ Clang-Tidy Checks :doc:`readability-redundant-string-cstr <readability/redundant-string-cstr>`, "Yes" :doc:`readability-redundant-string-init <readability/redundant-string-init>`, "Yes" :doc:`readability-reference-to-constructed-temporary <readability/reference-to-constructed-temporary>`, - :doc:`readability-return-expression-in-void-function <readability/return-expression-in-void-function>`, "Yes" + :doc:`readability-return-expression-in-void-function <readability/return-expression-in-void-function>`, :doc:`readability-simplify-boolean-expr <readability/simplify-boolean-expr>`, "Yes" :doc:`readability-simplify-subscript-expr <readability/simplify-subscript-expr>`, "Yes" :doc:`readability-static-accessed-through-instance <readability/static-accessed-through-instance>`, "Yes" From a4de6fd462af98e8286f38531aee20baae26b7eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 22 Dec 2023 18:19:57 +0100 Subject: [PATCH 5/9] Add non-triggering test cases --- .../return-expression-in-void-function.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp index eda7bcd36c2ea5..7398e29c7fb3d5 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp @@ -21,3 +21,14 @@ void f5() { return f4<void>(); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] } + +void f6() { return; } + +int f7() { return 1; } + +int f8() { return f7(); } + +void f9() { + return (void)f7(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] +} \ No newline at end of file From 5826c06ccbd4b8f7b10b80784f194d01ebf4c16e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 22 Dec 2023 18:32:32 +0100 Subject: [PATCH 6/9] Rename check --- ...Check.cpp => AvoidReturnWithVoidValueCheck.cpp} | 9 ++++----- ...tionCheck.h => AvoidReturnWithVoidValueCheck.h} | 14 +++++++------- .../clang-tidy/readability/CMakeLists.txt | 2 +- .../readability/ReadabilityTidyModule.cpp | 6 +++--- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- clang-tools-extra/docs/clang-tidy/checks/list.rst | 2 +- ...nction.rst => avoid-return-with-void-value.rst} | 6 +++--- ...nction.cpp => avoid-return-with-void-value.cpp} | 12 ++++++------ 8 files changed, 27 insertions(+), 28 deletions(-) rename clang-tools-extra/clang-tidy/readability/{ReturnExpressionInVoidFunctionCheck.cpp => AvoidReturnWithVoidValueCheck.cpp} (76%) rename clang-tools-extra/clang-tidy/readability/{ReturnExpressionInVoidFunctionCheck.h => AvoidReturnWithVoidValueCheck.h} (58%) rename clang-tools-extra/docs/clang-tidy/checks/readability/{return-expression-in-void-function.rst => avoid-return-with-void-value.rst} (77%) rename clang-tools-extra/test/clang-tidy/checkers/readability/{return-expression-in-void-function.cpp => avoid-return-with-void-value.cpp} (52%) diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp similarity index 76% rename from clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp rename to clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index 6517a4f8323c6b..377ce02dc3edf2 100644 --- a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -1,4 +1,4 @@ -//===--- ReturnExpressionInVoidFunctionCheck.cpp - clang-tidy -------------===// +//===--- AvoidReturnWithVoidValueCheck.cpp - clang-tidy -------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "ReturnExpressionInVoidFunctionCheck.h" +#include "AvoidReturnWithVoidValueCheck.h" #include "clang/AST/Stmt.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -15,14 +15,13 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { -void ReturnExpressionInVoidFunctionCheck::registerMatchers( - MatchFinder *Finder) { +void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( returnStmt(hasReturnValue(hasType(voidType()))).bind("void_return"), this); } -void ReturnExpressionInVoidFunctionCheck::check( +void AvoidReturnWithVoidValueCheck::check( const MatchFinder::MatchResult &Result) { const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); diag(VoidReturn->getBeginLoc(), diff --git a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h similarity index 58% rename from clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h rename to clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h index 39c9553d602728..209de70dfb06ff 100644 --- a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.h +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -1,4 +1,4 @@ -//===--- ReturnExpressionInVoidFunctionCheck.h - clang-tidy -----*- C++ -*-===// +//===--- AvoidReturnWithVoidValueCheck.h - clang-tidy -----------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDRETURNWITHVOIDVALUECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDRETURNWITHVOIDVALUECHECK_H #include "../ClangTidyCheck.h" @@ -16,10 +16,10 @@ namespace clang::tidy::readability { /// Looks for statements returning expressions of type `void`. /// /// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/readability/return-expression-in-void-function.html -class ReturnExpressionInVoidFunctionCheck : public ClangTidyCheck { +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-return-with-void-value.html +class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { public: - ReturnExpressionInVoidFunctionCheck(StringRef Name, ClangTidyContext *Context) + AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -30,4 +30,4 @@ class ReturnExpressionInVoidFunctionCheck : public ClangTidyCheck { } // namespace clang::tidy::readability -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNEXPRESSIONINVOIDFUNCTIONCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDRETURNWITHVOIDVALUECHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 7d0399ed901267..408c822b861c5f 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyReadabilityModule AvoidConstParamsInDecls.cpp + AvoidReturnWithVoidValueCheck.cpp AvoidUnconditionalPreprocessorIfCheck.cpp BracesAroundStatementsCheck.cpp ConstReturnTypeCheck.cpp @@ -42,7 +43,6 @@ add_clang_library(clangTidyReadabilityModule RedundantStringCStrCheck.cpp RedundantStringInitCheck.cpp ReferenceToConstructedTemporaryCheck.cpp - ReturnExpressionInVoidFunctionCheck.cpp SimplifyBooleanExprCheck.cpp SimplifySubscriptExprCheck.cpp StaticAccessedThroughInstanceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 2a9134071b9a72..0b0aad7c0dcb36 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "AvoidConstParamsInDecls.h" +#include "AvoidReturnWithVoidValueCheck.h" #include "AvoidUnconditionalPreprocessorIfCheck.h" #include "BracesAroundStatementsCheck.h" #include "ConstReturnTypeCheck.h" @@ -45,7 +46,6 @@ #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" #include "ReferenceToConstructedTemporaryCheck.h" -#include "ReturnExpressionInVoidFunctionCheck.h" #include "SimplifyBooleanExprCheck.h" #include "SimplifySubscriptExprCheck.h" #include "StaticAccessedThroughInstanceCheck.h" @@ -64,6 +64,8 @@ class ReadabilityModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<AvoidConstParamsInDecls>( "readability-avoid-const-params-in-decls"); + CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>( + "readability-avoid-return-with-void-value"); CheckFactories.registerCheck<AvoidUnconditionalPreprocessorIfCheck>( "readability-avoid-unconditional-preprocessor-if"); CheckFactories.registerCheck<BracesAroundStatementsCheck>( @@ -120,8 +122,6 @@ class ReadabilityModule : public ClangTidyModule { "readability-redundant-preprocessor"); CheckFactories.registerCheck<ReferenceToConstructedTemporaryCheck>( "readability-reference-to-constructed-temporary"); - CheckFactories.registerCheck<ReturnExpressionInVoidFunctionCheck>( - "readability-return-expression-in-void-function"); CheckFactories.registerCheck<SimplifySubscriptExprCheck>( "readability-simplify-subscript-expr"); CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 20896df0d340aa..147d3cc8bf1d21 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -218,8 +218,8 @@ New checks Detects C++ code where a reference variable is used to extend the lifetime of a temporary object that has just been constructed. -- New :doc:`readability-return-expression-in-void-function - <clang-tidy/checks/readability/return-expression-in-void-function>` check. +- New :doc:`readability-avoid-return-with-void-value + <clang-tidy/checks/readability/avoid-return-with-void-value>` check. Complains about statements returning expressions of type ``void``. It can be confusing if a function returns an expression even though its return type is diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index b32660aac7e6d0..06aa53a6e56cb5 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -336,6 +336,7 @@ Clang-Tidy Checks :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, :doc:`portability-std-allocator-const <portability/std-allocator-const>`, :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" + :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" :doc:`readability-const-return-type <readability/const-return-type>`, "Yes" @@ -371,7 +372,6 @@ Clang-Tidy Checks :doc:`readability-redundant-string-cstr <readability/redundant-string-cstr>`, "Yes" :doc:`readability-redundant-string-init <readability/redundant-string-init>`, "Yes" :doc:`readability-reference-to-constructed-temporary <readability/reference-to-constructed-temporary>`, - :doc:`readability-return-expression-in-void-function <readability/return-expression-in-void-function>`, :doc:`readability-simplify-boolean-expr <readability/simplify-boolean-expr>`, "Yes" :doc:`readability-simplify-subscript-expr <readability/simplify-subscript-expr>`, "Yes" :doc:`readability-static-accessed-through-instance <readability/static-accessed-through-instance>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst similarity index 77% rename from clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst rename to clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst index dd07b95550e6f8..e8e759b316da6b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/return-expression-in-void-function.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst @@ -1,7 +1,7 @@ -.. title:: clang-tidy - readability-return-expression-in-void-function +.. title:: clang-tidy - readability-avoid-return-with-void-value -readability-return-expression-in-void-function -============================================== +readability-avoid-return-with-void-value +======================================== Complains about statements returning expressions of type ``void``. It can be confusing if a function returns an expression even though its return type is diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp similarity index 52% rename from clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp rename to clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp index 7398e29c7fb3d5..e4ffa352b4ca7b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/return-expression-in-void-function.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp @@ -1,17 +1,17 @@ -// RUN: %check_clang_tidy %s readability-return-expression-in-void-function %t +// RUN: %check_clang_tidy %s readability-avoid-return-with-void-value %t void f1(); void f2() { return f1(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] } void f3(bool b) { if (b) return f1(); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] return f2(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] } template<class T> @@ -19,7 +19,7 @@ T f4() {} void f5() { return f4<void>(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] } void f6() { return; } @@ -30,5 +30,5 @@ int f8() { return f7(); } void f9() { return (void)f7(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-return-expression-in-void-function] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] } \ No newline at end of file From 3a87d499a39099f65778004fc74d41f461543a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 23 Dec 2023 17:08:38 +0100 Subject: [PATCH 7/9] Improve descriptions --- clang-tools-extra/docs/ReleaseNotes.rst | 5 ++--- .../readability/avoid-return-with-void-value.rst | 15 +++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 147d3cc8bf1d21..299a47c1d48e4b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -221,9 +221,8 @@ New checks - New :doc:`readability-avoid-return-with-void-value <clang-tidy/checks/readability/avoid-return-with-void-value>` check. - Complains about statements returning expressions of type ``void``. It can be - confusing if a function returns an expression even though its return type is - ``void``. + Finds return statements with ``void`` values used within functions with + ``void`` result types. New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst index e8e759b316da6b..92c987034c01c7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst @@ -3,9 +3,9 @@ readability-avoid-return-with-void-value ======================================== -Complains about statements returning expressions of type ``void``. It can be -confusing if a function returns an expression even though its return type is -``void``. +A function with a ``void`` return type is intended to perform a task without +producing a return value. Return statements with expressions could lead +to confusion and may miscommunicate the function's intended behavior. Example: @@ -17,9 +17,9 @@ Example: return g(); } -In a long function body, the ``return`` statements suggests that the function -returns a value. However, ``return g();`` is combination of two statements that -should be written as +In a long function body, the ``return`` statement suggests that the function +returns a value. However, ``return g();`` is a combination of two statements +that should be written as .. code-block:: @@ -28,3 +28,6 @@ should be written as to make clear that ``g()`` is called and immediately afterwards the function returns (nothing). + +In C, the same issue is detected by the compiler if the ``-Wpedantic`` mode +is enabled. From affa9d768fd6199b1bdc2fa669c1924f6d606cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 23 Dec 2023 17:08:58 +0100 Subject: [PATCH 8/9] Update message --- .../readability/AvoidReturnWithVoidValueCheck.cpp | 6 +++--- .../readability/avoid-return-with-void-value.cpp | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index 377ce02dc3edf2..5b90f7ab0a0625 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -1,4 +1,4 @@ -//===--- AvoidReturnWithVoidValueCheck.cpp - clang-tidy -------------===// +//===--- AvoidReturnWithVoidValueCheck.cpp - clang-tidy -------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -24,8 +24,8 @@ void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { void AvoidReturnWithVoidValueCheck::check( const MatchFinder::MatchResult &Result) { const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); - diag(VoidReturn->getBeginLoc(), - "return statement in void function should not return a value"); + diag(VoidReturn->getBeginLoc(), "return statement within a void function " + "should not have a specified return value"); } } // namespace clang::tidy::readability diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp index e4ffa352b4ca7b..3988be1a4d9000 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp @@ -4,14 +4,14 @@ void f1(); void f2() { return f1(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } void f3(bool b) { if (b) return f1(); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] return f2(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } template<class T> @@ -19,7 +19,7 @@ T f4() {} void f5() { return f4<void>(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } void f6() { return; } @@ -30,5 +30,5 @@ int f8() { return f7(); } void f9() { return (void)f7(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement in void function should not return a value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } \ No newline at end of file From 3984ddbcd5ebc8ec33eadf941770275c00ad54e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 23 Dec 2023 17:12:09 +0100 Subject: [PATCH 9/9] Restrict check to C++ --- .../clang-tidy/readability/AvoidReturnWithVoidValueCheck.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h index 209de70dfb06ff..c9331fe38e8c90 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -21,11 +21,17 @@ class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { public: AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: std::optional<TraversalKind> getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } }; } // namespace clang::tidy::readability _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits