[clang-tidy] implement check for goto

The usage of `goto` is discourage in C++ since forever. This check implements
a warning for every `goto`. Even though there are (rare) valid use cases for
`goto`, better high level constructs should be used.

`goto` is used sometimes in C programs to free resources at the end of 
functions in the case of errors. This pattern is better implemented with
RAII in C++.

Reviewers: aaron.ballman, alexfh, hokein

Reviewed By: aaron.ballman

Subscribers: lebedev.ri, jbcoe, Eugene.Zelenko, klimek, nemanjai, mgorny, xazax.hun, kbarton, cfe-commits

Differential Revision: https://reviews.llvm.org/D41815

llvm-svn: 322626

5 files changed, 98 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
new file mode 100644
index 00000000000..923555b90d8
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
@@ -0,0 +1,55 @@
+//===--- AvoidGotoCheck.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 "AvoidGotoCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+AST_MATCHER(GotoStmt, isForwardJumping) {
+  return Node.getLocStart() < Node.getLabel()->getLocStart();
+}
+
+void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  // TODO: This check does not recognize `IndirectGotoStmt` which is a
+  // GNU extension. These must be matched separately and an AST matcher
+  // is currently missing for them.
+
+  // Check if the 'goto' is used for control flow other than jumping
+  // out of a nested loop.
+  auto Loop = stmt(anyOf(forStmt(), cxxForRangeStmt(), whileStmt(), doStmt()));
+  auto NestedLoop =
+      stmt(anyOf(forStmt(hasAncestor(Loop)), cxxForRangeStmt(hasAncestor(Loop)),
+                 whileStmt(hasAncestor(Loop)), doStmt(hasAncestor(Loop))));
+
+  Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)),
+                                    unless(isForwardJumping())))
+                         .bind("goto"),
+                     this);
+}
+
+void AvoidGotoCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Goto = Result.Nodes.getNodeAs<GotoStmt>("goto");
+
+  diag(Goto->getGotoLoc(), "avoid using 'goto' for flow control")
+      << Goto->getSourceRange();
+  diag(Goto->getLabel()->getLocStart(), "label defined here",
+       DiagnosticIDs::Note);
+}
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
new file mode 100644
index 00000000000..cf7dd76f537
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
@@ -0,0 +1,36 @@
+//===--- AvoidGotoCheck.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_CPPCOREGUIDELINES_AVOIDGOTOCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDGOTOCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// The usage of ``goto`` for control flow is error prone and should be replaced
+/// with looping constructs. Only forward jumps in nested loops are accepted.
+//
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html
+class AvoidGotoCheck : public ClangTidyCheck {
+public:
+  AvoidGotoCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDGOTOCHECK_H
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index 70ee599c69e..2ce1a0800de 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
+  AvoidGotoCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
   NoMallocCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index 454adc9fd19..51824eca9ac 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
+#include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
@@ -35,6 +36,8 @@ namespace cppcoreguidelines {
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<AvoidGotoCheck>(
+        "cppcoreguidelines-avoid-goto");
     CheckFactories.registerCheck<InterfacesGlobalInitCheck>(
         "cppcoreguidelines-interfaces-global-init");
     CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
diff --git a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
index 0bb25836f14..89a1adaf401 100644
--- a/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../bugprone/UseAfterMoveCheck.h"
+#include "../cppcoreguidelines/AvoidGotoCheck.h"
 #include "../cppcoreguidelines/NoMallocCheck.h"
 #include "../cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h"
 #include "../cppcoreguidelines/ProTypeMemberInitCheck.h"
@@ -45,6 +46,8 @@ namespace hicpp {
 class HICPPModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<cppcoreguidelines::AvoidGotoCheck>(
+        "hicpp-avoid-goto");
     CheckFactories.registerCheck<readability::BracesAroundStatementsCheck>(
         "hicpp-braces-around-statements");
     CheckFactories.registerCheck<modernize::DeprecatedHeadersCheck>(