[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

4 files changed, 76 insertions, 0 deletions

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a60d372ccc9..1531545bd34 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -57,6 +57,13 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- New `cppcoreguidelines-avoid-goto
+  <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_ check
+
+  The usage of ``goto`` for control flow is error prone and should be replaced
+  with looping constructs. Every backward jump is rejected. Forward jumps are
+  only allowed in nested loops.
+
 - New `fuchsia-statically-constructed-objects
   <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-statically-constructed-objects.html>`_ check
 
@@ -64,6 +71,11 @@ Improvements to clang-tidy
   object is statically initialized with a ``constexpr`` constructor or has no 
   explicit constructor.
 
+- New alias `hicpp-avoid-goto
+  <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-avoid-goto.html>`_ to 
+  `cppcoreguidelines-avoid-goto <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_
+  added.
+
 Improvements to include-fixer
 -----------------------------
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
new file mode 100644
index 00000000000..fcb7dd31325
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-goto
+
+cppcoreguidelines-avoid-goto
+============================
+
+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.
+
+This check implements `ES.76 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es76-avoid-goto>`_ 
+from the CppCoreGuidelines and 
+`6.3.1 from High Integrity C++ <http://www.codingstandard.com/rule/6-3-1-ensure-that-the-labels-for-a-jump-statement-or-a-switch-condition-appear-later-in-the-same-or-an-enclosing-block/>`_.
+
+For more information on why to avoid programming 
+with ``goto`` you can read the famous paper `A Case against the GO TO Statement. <https://www.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF>`_.
+
+The check diagnoses ``goto`` for backward jumps in every language mode. These
+should be replaced with `C/C++` looping constructs.
+
+.. code-block:: c++
+
+  // Bad, handwritten for loop.
+  int i = 0;
+  // Jump label for the loop
+  loop_start:
+  do_some_operation();
+
+  if (i < 100) {
+    ++i;
+    goto loop_start;
+  }
+
+  // Better
+  for(int i = 0; i < 100; ++i)
+    do_some_operation();
+
+Modern C++ needs ``goto`` only to jump out of nested loops.
+
+.. code-block:: c++
+
+  for(int i = 0; i < 100; ++i) {
+    for(int j = 0; j < 100; ++j) {
+      if (i * j > 500)
+        goto early_exit;
+    }
+  }
+
+  early_exit:
+  some_operation();
+
+All other uses of ``goto`` are diagnosed in `C++`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/hicpp-avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/hicpp-avoid-goto.rst
new file mode 100644
index 00000000000..f689bec6f53
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/hicpp-avoid-goto.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - hicpp-avoid-goto
+
+hicpp-avoid-goto
+================
+
+The `hicpp-avoid-goto` check is an alias to 
+`cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_.
+Rule `6.3.1 High Integrity C++ <http://www.codingstandard.com/rule/6-3-1-ensure-that-the-labels-for-a-jump-statement-or-a-switch-condition-appear-later-in-the-same-or-an-enclosing-block/>`_
+requires that ``goto`` only skips parts of a block and is not used for other 
+reasons.
+
+Both coding guidelines implement the same exception to the usage of ``goto``.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 9b3e717c6f9..77b398b01ff 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -52,6 +52,7 @@ Clang-Tidy Checks
    cert-msc30-c (redirects to cert-msc50-cpp) <cert-msc30-c>
    cert-msc50-cpp
    cert-oop11-cpp (redirects to performance-move-constructor-init) <cert-oop11-cpp>
+   cppcoreguidelines-avoid-goto
    cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
    cppcoreguidelines-interfaces-global-init
    cppcoreguidelines-no-malloc
@@ -90,6 +91,7 @@ Clang-Tidy Checks
    google-runtime-member-string-references
    google-runtime-operator
    google-runtime-references
+   hicpp-avoid-goto (redirects to cppcoreguidelines-avoid-goto) <hicpp-avoid-goto>
    hicpp-braces-around-statements (redirects to readability-braces-around-statements) <hicpp-braces-around-statements>
    hicpp-deprecated-headers (redirects to modernize-deprecated-headers) <hicpp-deprecated-headers>
    hicpp-exception-baseclass