[clang-tools-extra] r322626 - [clang-tidy] implement check for goto
Jonas Toth via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 17 02:27:41 PST 2018
Author: jonastoth
Date: Wed Jan 17 02:27:41 2018
New Revision: 322626
URL: http://llvm.org/viewvc/llvm-project?rev=322626&view=rev
Log:
[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
Added:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-avoid-goto.rst
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp?rev=322626&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp Wed Jan 17 02:27:41 2018
@@ -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
Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h?rev=322626&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h Wed Jan 17 02:27:41 2018
@@ -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
Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=322626&r1=322625&r2=322626&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Wed Jan 17 02:27:41 2018
@@ -1,6 +1,7 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyCppCoreGuidelinesModule
+ AvoidGotoCheck.cpp
CppCoreGuidelinesTidyModule.cpp
InterfacesGlobalInitCheck.cpp
NoMallocCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=322626&r1=322625&r2=322626&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp Wed Jan 17 02:27:41 2018
@@ -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");
Modified: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp?rev=322626&r1=322625&r2=322626&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp Wed Jan 17 02:27:41 2018
@@ -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>(
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=322626&r1=322625&r2=322626&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Jan 17 02:27:41 2018
@@ -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
-----------------------------
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst?rev=322626&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst Wed Jan 17 02:27:41 2018
@@ -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++`.
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-avoid-goto.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-avoid-goto.rst?rev=322626&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-avoid-goto.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-avoid-goto.rst Wed Jan 17 02:27:41 2018
@@ -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``.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=322626&r1=322625&r2=322626&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Jan 17 02:27:41 2018
@@ -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
Added: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-avoid-goto.cpp?rev=322626&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-avoid-goto.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-avoid-goto.cpp Wed Jan 17 02:27:41 2018
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t
+
+void noop() {}
+
+int main() {
+ noop();
+ goto jump_to_me;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-MESSAGES: [[@LINE+3]]:1: note: label defined here
+ noop();
+
+jump_to_me:;
+
+jump_backwards:;
+ noop();
+ goto jump_backwards;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-MESSAGES: [[@LINE-4]]:1: note: label defined here
+
+ goto jump_in_line;
+ ;
+jump_in_line:;
+ // CHECK-MESSAGES: [[@LINE-3]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-MESSAGES: [[@LINE-2]]:1: note: label defined here
+
+ // Test the GNU extension https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
+some_label:;
+ void *dynamic_label = &&some_label;
+
+ // FIXME: `IndirectGotoStmt` is not detected.
+ goto *dynamic_label;
+}
+
+void forward_jump_out_nested_loop() {
+ int array[] = {1, 2, 3, 4, 5};
+ for (int i = 0; i < 10; ++i) {
+ noop();
+ for (int j = 0; j < 10; ++j) {
+ noop();
+ if (i + j > 10)
+ goto early_exit1;
+ }
+ noop();
+ }
+
+ for (int i = 0; i < 10; ++i) {
+ noop();
+ while (true) {
+ noop();
+ if (i > 5)
+ goto early_exit1;
+ }
+ noop();
+ }
+
+ for (auto value : array) {
+ noop();
+ for (auto number : array) {
+ noop();
+ if (number == 5)
+ goto early_exit1;
+ }
+ }
+
+ do {
+ noop();
+ do {
+ noop();
+ goto early_exit1;
+ } while (true);
+ } while (true);
+
+ do {
+ for (auto number : array) {
+ noop();
+ if (number == 2)
+ goto early_exit1;
+ }
+ } while (true);
+
+ // Jumping further results in error, because the variable declaration would
+ // be skipped.
+early_exit1:;
+
+ int i = 0;
+ while (true) {
+ noop();
+ while (true) {
+ noop();
+ if (i > 5)
+ goto early_exit2;
+ i++;
+ }
+ noop();
+ }
+
+ while (true) {
+ noop();
+ for (int j = 0; j < 10; ++j) {
+ noop();
+ if (j > 5)
+ goto early_exit2;
+ }
+ noop();
+ }
+
+ while (true) {
+ noop();
+ for (auto number : array) {
+ if (number == 1)
+ goto early_exit2;
+ noop();
+ }
+ }
+
+ while (true) {
+ noop();
+ do {
+ noop();
+ goto early_exit2;
+ } while (true);
+ }
+early_exit2:;
+}
+
+void jump_out_backwards() {
+
+before_the_loop:
+ noop();
+
+ for (int i = 0; i < 10; ++i) {
+ for (int j = 0; j < 10; ++j) {
+ if (i * j > 80)
+ goto before_the_loop;
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control
+ // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here
+ }
+ }
+}
More information about the cfe-commits
mailing list