Add a new check, readability-redundant-control-flow, that check for some forms of redundant control flow statements. Currently checks for return statements at the end of a function with a void return type and continue statements at the end of looping statements.

Patch by Richard Thomson.

llvm-svn: 259362

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityModule
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
+  RedundantControlFlowCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   SimplifyBooleanExprCheck.cpp

clang-tools-extra/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 @@ public:
         "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    CheckFactories.registerCheck<RedundantControlFlowCheck>(
+        "readability-redundant-control-flow");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(

clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp

@@ -0,0 +1,99 @@
+//===--- 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 a function with a void return type";
+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(
+      stmt(anyOf(forStmt(), cxxForRangeStmt(), whileStmt(), doStmt()),
+           CompoundContinue),
+      this);
+}
+
+void RedundantControlFlowCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Return = Result.Nodes.getNodeAs<CompoundStmt>("return"))
+    checkRedundantReturn(Result, Return);
+  else if (const auto *Continue =
+               Result.Nodes.getNodeAs<CompoundStmt>("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<ReturnStmt>(*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<ContinueStmt>(*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<Stmt>(*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

clang-tools-extra/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

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -86,6 +86,7 @@ Clang-Tidy Checks
    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

clang-tools-extra/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) {
+    }
+  }

clang-tools-extra/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 a function with a void return type [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 <typename T>
+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 <typename T>
+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);
+}