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/5] [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 f4029bb6946d10ed4eed5beeee54d6dce420e1cf 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/5] Clarify check message --- .../readability/ReturnExpressionInVoidFunctionCheck.cpp | 2 +- .../readability/return-expression-in-void-function.cpp | 8 ++++---- 2 files changed, 5 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..79c2cd3a1717ea 100644 --- a/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReturnExpressionInVoidFunctionCheck.cpp @@ -25,7 +25,7 @@ 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 936fe2434fe0a70a96fc4e1e9ef402cf43d3e523 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/5] 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 e5a086a7db61935a7c9eb0d1724c3f9377bbb80b 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/5] 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 8437ac7d0cebbbb3d28e94b37df5a00944356dd1 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/5] 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits