Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -11,6 +11,7 @@ NamedParameterCheck.cpp NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp + RedundantControlFlowCheck.cpp RedundantStringCStrCheck.cpp RedundantSmartptrGetCheck.cpp SimplifyBooleanExprCheck.cpp Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -18,6 +18,7 @@ #include "ImplicitBoolCastCheck.h" #include "InconsistentDeclarationParameterNameCheck.h" #include "NamedParameterCheck.h" +#include "RedundantControlFlowCheck.h" #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "SimplifyBooleanExprCheck.h" @@ -44,6 +45,8 @@ "readability-implicit-bool-cast"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-redundant-control-flow"); CheckFactories.registerCheck( "readability-uniqueptr-delete-release"); CheckFactories.registerCheck( Index: clang-tidy/readability/RedundantControlFlowCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantControlFlowCheck.h @@ -0,0 +1,51 @@ +//===--- RedundantControlFlowCheck.h - clang-tidy----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_CONTROL_FLOW_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_CONTROL_FLOW_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Eliminates redundant `return` statements at the end of a function that +/// returns `void`. +/// +/// Eliminates redundant `continue` statements at the end of a loop body. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-control-flow.html +class RedundantControlFlowCheck : public ClangTidyCheck { +public: + RedundantControlFlowCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + void + checkRedundantReturn(const ast_matchers::MatchFinder::MatchResult &Result, + const CompoundStmt *Block); + + void + checkRedundantContinue(const ast_matchers::MatchFinder::MatchResult &Result, + const CompoundStmt *Block); + + void issueDiagnostic(const ast_matchers::MatchFinder::MatchResult &Result, + const CompoundStmt *Block, const SourceRange &StmtRange, + const char *Diag); +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_CONTROL_FLOW_H Index: clang-tidy/readability/RedundantControlFlowCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantControlFlowCheck.cpp @@ -0,0 +1,102 @@ +//===--- RedundantControlFlowCheck.cpp - clang-tidy------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "RedundantControlFlowCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { + +const char *const RedundantReturnDiag = + "redundant return statement at the end of void function"; +const char *const RedundantContinueDiag = + "redundant continue statement at the end of loop statement"; + +bool isLocationInMacroExpansion(const SourceManager &SM, SourceLocation Loc) { + return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc); +} + +} // namespace + +void RedundantControlFlowCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(isDefinition(), returns(voidType()), + has(compoundStmt(hasAnySubstatement(returnStmt( + unless(has(expr()))))).bind("return"))), + this); + auto CompoundContinue = + has(compoundStmt(hasAnySubstatement(continueStmt())).bind("continue")); + Finder->addMatcher(forStmt(CompoundContinue), this); + Finder->addMatcher(cxxForRangeStmt(CompoundContinue), this); + Finder->addMatcher(whileStmt(CompoundContinue), this); + Finder->addMatcher(doStmt(CompoundContinue), this); +} + +void RedundantControlFlowCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *Return = Result.Nodes.getNodeAs("return")) + checkRedundantReturn(Result, Return); + else if (const auto *Continue = + Result.Nodes.getNodeAs("continue")) + checkRedundantContinue(Result, Continue); +} + +void RedundantControlFlowCheck::checkRedundantReturn( + const MatchFinder::MatchResult &Result, const CompoundStmt *Block) { + CompoundStmt::const_reverse_body_iterator last = Block->body_rbegin(); + if (const auto *Return = dyn_cast(*last)) { + issueDiagnostic(Result, Block, Return->getSourceRange(), + RedundantReturnDiag); + } +} + +void RedundantControlFlowCheck::checkRedundantContinue( + const MatchFinder::MatchResult &Result, const CompoundStmt *Block) { + CompoundStmt::const_reverse_body_iterator last = Block->body_rbegin(); + if (const auto *Continue = dyn_cast(*last)) { + issueDiagnostic(Result, Block, Continue->getSourceRange(), + RedundantContinueDiag); + } +} + +void RedundantControlFlowCheck::issueDiagnostic( + const MatchFinder::MatchResult &Result, const CompoundStmt *const Block, + const SourceRange &StmtRange, const char *const Diag) { + SourceManager &SM = *Result.SourceManager; + if (isLocationInMacroExpansion(SM, StmtRange.getBegin())) + return; + + CompoundStmt::const_reverse_body_iterator Previous = ++Block->body_rbegin(); + SourceLocation Start; + if (Previous != Block->body_rend()) { + Start = Lexer::findLocationAfterToken( + dyn_cast(*Previous)->getLocEnd(), tok::semi, SM, + Result.Context->getLangOpts(), + /*SkipTrailingWhitespaceAndNewLine=*/true); + } else { + Start = StmtRange.getBegin(); + } + auto RemovedRange = CharSourceRange::getCharRange( + Start, + Lexer::findLocationAfterToken(StmtRange.getEnd(), tok::semi, SM, + Result.Context->getLangOpts(), + /*SkipTrailingWhitespaceAndNewLine=*/true)); + + diag(StmtRange.getBegin(), Diag) << FixItHint::CreateRemoval(RemovedRange); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -84,6 +84,7 @@ readability-implicit-bool-cast readability-inconsistent-declaration-parameter-name readability-named-parameter + readability-redundant-control-flow readability-redundant-smartptr-get readability-redundant-string-cstr readability-simplify-boolean-expr Index: docs/clang-tidy/checks/readability-redundant-control-flow.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-control-flow.rst @@ -0,0 +1,50 @@ +.. title:: clang-tidy - readability-redundant-control-flow + +readability-redundant-control-flow +================================== + +This check looks for procedures (functions returning no value) with `return` +statements at the end of the function. Such `return` statements are redundant. + +Loop statements (`for`, `while`, `do while`) are checked for redundant +`continue` statements at the end of the loop body. + +Examples: + +The following function `f` contains a redundant `return` statement: + +.. code:: c++ + + extern void g(); + void f() { + g(); + return; + } + +becomes + +.. code:: c++ + + extern void g(); + void f() { + g(); + } + +The following function `k` contains a redundant `continue` statement: + +.. code:: c++ + + void k() { + for (int i = 0; i < 10; ++i) { + continue; + } + } + +becomes + +.. code:: c++ + + void k() { + for (int i = 0; i < 10; ++i) { + } + } Index: test/clang-tidy/readability-redundant-control-flow.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-redundant-control-flow.cpp @@ -0,0 +1,222 @@ +// RUN: %check_clang_tidy %s readability-redundant-control-flow %t + +void g(int i); +void j(); + +void f() { + return; +} +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: redundant return statement at the end of void function [readability-redundant-control-flow] +// CHECK-FIXES: {{^}}void f() {{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +void g() { + f(); + return; +} +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: redundant return statement +// CHECK-FIXES: {{^ }}f();{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +void g(int i) { + if (i < 0) { + return; + } + if (i < 10) { + f(); + } +} + +int h() { + return 1; +} + +void j() { +} + +void k() { + for (int i = 0; i < 10; ++i) { + continue; + } +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement at the end of loop statement +// CHECK-FIXES: {{^}} for (int i = 0; i < 10; ++i) {{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +void k2() { + int v[10] = { 0 }; + for (auto i : v) { + continue; + } +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement +// CHECK-FIXES: {{^}} for (auto i : v) {{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +void m() { + int i = 0; + do { + ++i; + continue; + } while (i < 10); +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement +// CHECK-FIXES: {{^ do {$}} +// CHECK-FIXES-NEXT: {{^}} ++i;{{$}} +// CHECK-FIXES-NEXT: {{^ *}}} while (i < 10);{{$}} + +void p() { + int i = 0; + while (i < 10) { + ++i; + continue; + } +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement +// CHECK-FIXES: {{^}} while (i < 10) {{{$}} +// CHECK-FIXES-NEXT: {{^}} ++i;{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +void im_not_dead(int i) { + if (i > 0) { + return; + } + g(); +} + +void im_still_not_dead(int i) { + for (int j = 0; j < 10; ++j) { + if (i < 10) { + continue; + } + g(); + } +} + +void im_dead(int i) { + if (i > 0) { + return; + g(); + } + g(); +} + +void im_still_dead(int i) { + for (int j = 0; j < 10; ++j) { + if (i < 10) { + continue; + g(); + } + g(); + } +} + +void void_return() { + return g(); +} + +void nested_return_unmolested() { + g(); + { + g(); + return; + } +} + +void nested_continue_unmolested() { + for (int i = 0; i < 10; ++i) { + if (i < 5) { + continue; + } + } +} + +#define MACRO_RETURN_UNMOLESTED(fn_) \ + (fn_)(); \ + return + +#define MACRO_CONTINUE_UNMOLESTED(x_) \ + do { \ + for (int i = 0; i < (x_); ++i) { \ + continue; \ + } \ + } while (false) + +void macro_return() { + MACRO_RETURN_UNMOLESTED(g); +} + +void macro_continue() { + MACRO_CONTINUE_UNMOLESTED(10); +} + +#define MACRO_RETURN_ARG(stmt_) \ + stmt_ + +#define MACRO_CONTINUE_ARG(stmt_) \ + do { \ + for (int i = 0; i < 10; ++i) { \ + stmt_; \ + } \ + } while (false) + +void macro_arg_return() { + MACRO_RETURN_ARG(return); +} + +void macro_arg_continue() { + MACRO_CONTINUE_ARG(continue); +} + +template +void template_return(T check) { + if (check < T(0)) { + return; + } + return; +} +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: redundant return statement +// CHECK-FIXES: {{^}} if (check < T(0)) {{{$}} +// CHECK-FIXES-NEXT: {{^ return;$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +template <> +void template_return(int check) { + if (check < 0) { + return; + } + return; +} +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: redundant return statement +// CHECK-FIXES: {{^}} if (check < 0) {{{$}} +// CHECK-FIXES-NEXT: {{^ return;$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +template +void template_loop(T end) { + for (T i = 0; i < end; ++i) { + continue; + } +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement +// CHECK-FIXES: {{^}} for (T i = 0; i < end; ++i) {{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +template <> +void template_loop(int end) { + for (int i = 0; i < end; ++i) { + continue; + } +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement +// CHECK-FIXES: {{^}} for (int i = 0; i < end; ++i) {{{$}} +// CHECK-FIXES-NEXT: {{^ *}$}} + +void call_templates() { + template_return(10); + template_return(10.0f); + template_return(10.0); + template_loop(10); + template_loop(10L); + template_loop(10U); +}