https://github.com/SimplyDanny updated https://github.com/llvm/llvm-project/pull/76249
From 16b877e782951293a67a819441a3910f19bc24ad 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 01/16] [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 4d25e2ebe85f5f..515b9311eef252 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -230,6 +230,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 b36bf7d497b9dd..c0410b0662cad5 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -372,6 +372,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 bb03f3f415d2d758d39cafa4ec18251047247def 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 02/16] 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 4b765d51cefc1f8fca4bff422ee96cd4d87de66d 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 03/16] 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 11f46b9de9914613149ec0fee4973d8da761da3e 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 04/16] 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 c0410b0662cad5..6b9528379e9295 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -372,7 +372,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 118f34f334ded0e11a1025215e09afbab936693e 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 05/16] 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 9c55840f6c27c04b1a2a1e4a24f03ac50b680ae2 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 06/16] 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 515b9311eef252..7936d10ee432cf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -230,8 +230,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 6b9528379e9295..2f86121ad87299 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -337,6 +337,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" @@ -372,7 +373,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 07f65d0147e61992b49a12ac541d326f8118133d 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 07/16] 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 7936d10ee432cf..08ade306b5a077 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -233,9 +233,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 309f4a85c52e8d1190d57d290c84e1706c783723 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 08/16] 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 bed0478cbfd942964269214b9f256b2bb828ed36 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 09/16] 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 From fa7ff39adb7629fabbb706721b6aa86a4b1a142b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 24 Dec 2023 14:14:57 +0100 Subject: [PATCH 10/16] Sync first sentence in release notes, description and class doc --- .../clang-tidy/readability/AvoidReturnWithVoidValueCheck.h | 3 ++- .../checks/readability/avoid-return-with-void-value.rst | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h index c9331fe38e8c90..48c42a12722b68 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -13,7 +13,8 @@ namespace clang::tidy::readability { -/// Looks for statements returning expressions of type `void`. +/// Finds return statements with `void` values used within functions with `void` +/// result types. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-return-with-void-value.html 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 92c987034c01c7..760b62cef31999 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,6 +3,9 @@ readability-avoid-return-with-void-value ======================================== +Finds return statements with ``void`` values used within functions with +``void`` result types. + 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. From 34f29e49e798cf91f8fd63854e844df5123d0d4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Tue, 26 Dec 2023 22:01:22 +0100 Subject: [PATCH 11/16] Add `IgnoreMacros` option --- .../readability/AvoidReturnWithVoidValueCheck.cpp | 2 ++ .../readability/AvoidReturnWithVoidValueCheck.h | 13 ++++++++++++- .../readability/avoid-return-with-void-value.rst | 8 ++++++++ .../readability/avoid-return-with-void-value.cpp | 12 +++++++++++- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index 5b90f7ab0a0625..47d5c540a69b78 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -24,6 +24,8 @@ void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { void AvoidReturnWithVoidValueCheck::check( const MatchFinder::MatchResult &Result) { const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); + if (IgnoreMacros && VoidReturn->getBeginLoc().isMacroID()) + return; diag(VoidReturn->getBeginLoc(), "return statement within a void function " "should not have a specified return value"); } diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h index 48c42a12722b68..94f3a8614f2d09 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -11,6 +11,9 @@ #include "../ClangTidyCheck.h" +static constexpr auto IgnoreMacrosName = "IgnoreMacros"; +static constexpr auto IgnoreMacrosDefault = true; + namespace clang::tidy::readability { /// Finds return statements with `void` values used within functions with `void` @@ -21,7 +24,9 @@ namespace clang::tidy::readability { class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { public: AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + IgnoreMacros( + Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -33,6 +38,12 @@ class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override { + Options.store(Opts, IgnoreMacrosName, IgnoreMacros); + } + +private: + bool IgnoreMacros; }; } // namespace clang::tidy::readability 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 760b62cef31999..93ed0f2aa53f40 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 @@ -34,3 +34,11 @@ returns (nothing). In C, the same issue is detected by the compiler if the ``-Wpedantic`` mode is enabled. + +Options +------- + +.. option:: IgnoreMacros + + The value ``false`` specifies that return statements expanded + from macros are not checked. The default value is ``true``. 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 3988be1a4d9000..5ec774d6c7a234 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 @@ -1,4 +1,7 @@ // RUN: %check_clang_tidy %s readability-avoid-return-with-void-value %t +// RUN: %check_clang_tidy -check-suffixes=,INCLUDE-MACROS %s readability-avoid-return-with-void-value %t \ +// RUN: -- -config="{CheckOptions: [{key: readability-avoid-return-with-void-value.IgnoreMacros, value: false}]}" \ +// RUN: -- void f1(); @@ -31,4 +34,11 @@ int f8() { return f7(); } void f9() { return (void)f7(); // 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 +} + +#define RETURN_VOID return (void)1 + +void f10() { + RETURN_VOID; + // CHECK-MESSAGES-INCLUDE-MACROS: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} From e1074de7a72a843b057980ad90b76d1b7a0b26fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Tue, 26 Dec 2023 22:52:08 +0100 Subject: [PATCH 12/16] Add `StrictMode` option --- .../readability/AvoidReturnWithVoidValueCheck.cpp | 6 +++++- .../readability/AvoidReturnWithVoidValueCheck.h | 9 ++++++++- .../checks/readability/avoid-return-with-void-value.rst | 7 +++++++ .../readability/avoid-return-with-void-value.cpp | 7 +++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index 47d5c540a69b78..6719e1fe34d857 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -17,7 +17,9 @@ namespace clang::tidy::readability { void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - returnStmt(hasReturnValue(hasType(voidType()))).bind("void_return"), + returnStmt(hasReturnValue(hasType(voidType())), + optionally(hasParent(compoundStmt().bind("compound_parent")))) + .bind("void_return"), this); } @@ -26,6 +28,8 @@ void AvoidReturnWithVoidValueCheck::check( const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); if (IgnoreMacros && VoidReturn->getBeginLoc().isMacroID()) return; + if (!StrictMode && !Result.Nodes.getNodeAs<CompoundStmt>("compound_parent")) + return; diag(VoidReturn->getBeginLoc(), "return statement within a void function " "should not have a specified return value"); } diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h index 94f3a8614f2d09..7bc769c695b336 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -14,6 +14,9 @@ static constexpr auto IgnoreMacrosName = "IgnoreMacros"; static constexpr auto IgnoreMacrosDefault = true; +static constexpr auto StrictModeName = "StrictMode"; +static constexpr auto StrictModeDefault = true; + namespace clang::tidy::readability { /// Finds return statements with `void` values used within functions with `void` @@ -26,7 +29,9 @@ class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IgnoreMacros( - Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)) {} + Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)), + StrictMode( + Options.getLocalOrGlobal(StrictModeName, StrictModeDefault)) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -40,10 +45,12 @@ class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { } void storeOptions(ClangTidyOptions::OptionMap &Opts) override { Options.store(Opts, IgnoreMacrosName, IgnoreMacros); + Options.store(Opts, StrictModeName, StrictMode); } private: bool IgnoreMacros; + bool StrictMode; }; } // namespace clang::tidy::readability 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 93ed0f2aa53f40..53169f68a61afb 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 @@ -42,3 +42,10 @@ Options The value ``false`` specifies that return statements expanded from macros are not checked. The default value is ``true``. + +.. option:: StrictMode + + The value ``false`` specifies that a direct return statement shall + be excluded from the analysis if it is the only statement not + contained in a block like ``if (cond) return g();``. The default + value is ``true``. 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 5ec774d6c7a234..9e511f49a64dc1 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 @@ -2,12 +2,16 @@ // RUN: %check_clang_tidy -check-suffixes=,INCLUDE-MACROS %s readability-avoid-return-with-void-value %t \ // RUN: -- -config="{CheckOptions: [{key: readability-avoid-return-with-void-value.IgnoreMacros, value: false}]}" \ // RUN: -- +// RUN: %check_clang_tidy -check-suffixes=LENIENT %s readability-avoid-return-with-void-value %t \ +// RUN: -- -config="{CheckOptions: [{key: readability-avoid-return-with-void-value.StrictMode, value: false}]}" \ +// RUN: -- void f1(); void f2() { return f1(); // 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] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]: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) { @@ -15,6 +19,7 @@ void f3(bool b) { // 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 within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } template<class T> @@ -23,6 +28,7 @@ T f4() {} void f5() { return f4<void>(); // 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] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } void f6() { return; } @@ -34,6 +40,7 @@ int f8() { return f7(); } void f9() { return (void)f7(); // 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] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } #define RETURN_VOID return (void)1 From 0b693996e386f36ca533b1dd0e12265f25758ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Wed, 27 Dec 2023 22:04:12 +0100 Subject: [PATCH 13/16] Ignore initializer lists seen as returning `void` --- .../readability/AvoidReturnWithVoidValueCheck.cpp | 5 +++-- .../checkers/readability/avoid-return-with-void-value.cpp | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index 6719e1fe34d857..29f712f5cf5b0f 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -17,8 +17,9 @@ namespace clang::tidy::readability { void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - returnStmt(hasReturnValue(hasType(voidType())), - optionally(hasParent(compoundStmt().bind("compound_parent")))) + returnStmt( + hasReturnValue(allOf(hasType(voidType()), unless(initListExpr()))), + optionally(hasParent(compoundStmt().bind("compound_parent")))) .bind("void_return"), this); } 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 9e511f49a64dc1..b63c83952396a5 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 @@ -49,3 +49,11 @@ void f10() { RETURN_VOID; // CHECK-MESSAGES-INCLUDE-MACROS: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } + +template <typename A> +struct C { + C(A) {} +}; + +template <class T> +C<T> f11() { return {}; } From d57060b2bf72c7db45c5ed10a3d095ecccfcb9f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 5 Jan 2024 22:14:20 +0100 Subject: [PATCH 14/16] Use single quotes for option values --- .../checks/readability/avoid-return-with-void-value.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 53169f68a61afb..d802f9be829c46 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 @@ -40,12 +40,12 @@ Options .. option:: IgnoreMacros - The value ``false`` specifies that return statements expanded - from macros are not checked. The default value is ``true``. + The value `false` specifies that return statements expanded + from macros are not checked. The default value is `true`. .. option:: StrictMode - The value ``false`` specifies that a direct return statement shall + The value `false` specifies that a direct return statement shall be excluded from the analysis if it is the only statement not contained in a block like ``if (cond) return g();``. The default - value is ``true``. + value is `true`. From cc9fc2f39b849299894bb3d86d78a7fd22a95ae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 5 Jan 2024 22:14:57 +0100 Subject: [PATCH 15/16] Move complex implementations to source file --- .../AvoidReturnWithVoidValueCheck.cpp | 19 +++++++++++++++++++ .../AvoidReturnWithVoidValueCheck.h | 18 ++---------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index 29f712f5cf5b0f..e3400f614fa564 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -15,6 +15,19 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { +static constexpr auto IgnoreMacrosName = "IgnoreMacros"; +static constexpr auto IgnoreMacrosDefault = true; + +static constexpr auto StrictModeName = "StrictMode"; +static constexpr auto StrictModeDefault = true; + +AvoidReturnWithVoidValueCheck::AvoidReturnWithVoidValueCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreMacros( + Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)), + StrictMode(Options.getLocalOrGlobal(StrictModeName, StrictModeDefault)) {} + void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( returnStmt( @@ -35,4 +48,10 @@ void AvoidReturnWithVoidValueCheck::check( "should not have a specified return value"); } +void AvoidReturnWithVoidValueCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, IgnoreMacrosName, IgnoreMacros); + Options.store(Opts, StrictModeName, StrictMode); +} + } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h index 7bc769c695b336..f8148db43cd952 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -11,12 +11,6 @@ #include "../ClangTidyCheck.h" -static constexpr auto IgnoreMacrosName = "IgnoreMacros"; -static constexpr auto IgnoreMacrosDefault = true; - -static constexpr auto StrictModeName = "StrictMode"; -static constexpr auto StrictModeDefault = true; - namespace clang::tidy::readability { /// Finds return statements with `void` values used within functions with `void` @@ -26,12 +20,7 @@ namespace clang::tidy::readability { /// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-return-with-void-value.html class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { public: - AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IgnoreMacros( - Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)), - StrictMode( - Options.getLocalOrGlobal(StrictModeName, StrictModeDefault)) {} + AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -43,10 +32,7 @@ class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } - void storeOptions(ClangTidyOptions::OptionMap &Opts) override { - Options.store(Opts, IgnoreMacrosName, IgnoreMacros); - Options.store(Opts, StrictModeName, StrictMode); - } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: bool IgnoreMacros; From 8725c560b1e4e832dbf990ce128fd8aa9c0b6ff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Fri, 5 Jan 2024 22:41:36 +0100 Subject: [PATCH 16/16] Add example for aliased `void` --- .../readability/avoid-return-with-void-value.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 b63c83952396a5..f00407c99ce570 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 @@ -57,3 +57,13 @@ struct C { template <class T> C<T> f11() { return {}; } + +using VOID = void; + +VOID f12(); + +VOID f13() { + return f12(); + // 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] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits