[clang-tidy] cppcoreguidelines-avoid-do-while: a new check

[DNS320]

Summary

This simple check flags do-while statements according to rule ES.75 of the C++ Core Guidelines.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

114

Please highlight do-while with double back-ticks.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst

7

Please highlight do-while with double back-ticks.

9

Please highlight do-while and while with double back-ticks.

[njames93]

This is going to be very noisey for codebases that use a do...while macro:

#define MACRO() do { Something(); With(); Multiple(); Statements(); } while (false)

Maybe you should ignore do statements that are in macro expansions as well as potentially do statements with a false condition.

[DNS320]

I updated the check according to the last review.

It ignores now do-while statements which are inside a macro and have a false condition. (false condition part was taken from bugprone-terminating-continue)

Tests were added and the check documentation has now a hint about ignoring do-while statements in macros.

[DNS320]

Friendly Ping.

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h

+//===--- AvoidDoWhileCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDDOWHILECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDDOWHILECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Checks if a do-while loop exists and flags it.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-do-while.html
+class AvoidDoWhileCheck : public ClangTidyCheck {
+public:
+  AvoidDoWhileCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  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_AVOIDDOWHILECHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp

+//===--- AvoidDoWhileCheck.cpp - clang-tidy -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AvoidDoWhileCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+AST_MATCHER(DoStmt, isInMacro) { return Node.getBeginLoc().isMacroID(); }
+
+void AvoidDoWhileCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      doStmt(unless(allOf(
+                 isInMacro(),
+                 hasCondition(ignoringImpCasts(anyOf(
+                     cxxBoolLiteral(equals(false)), integerLiteral(equals(0)),
+                     cxxNullPtrLiteralExpr(), gnuNullExpr()))))))
+          .bind("doStmt"),
+      this);
+}
+
+void AvoidDoWhileCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDoStmt = Result.Nodes.getNodeAs<DoStmt>("doStmt");
+
+  diag(MatchedDoStmt->getDoLoc(),
+       "Try to avoid using do-while loops in terms of readability. Consider "
+       "using a while loop instead.");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

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

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
+  AvoidDoWhileCheck.cpp
   AvoidGotoCheck.cpp
   AvoidNonConstGlobalVariablesCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InitVariablesCheck.cpp
   InterfacesGlobalInitCheck.cpp
   MacroUsageCheck.cpp
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 //===-- CppCoreGuidelinesTidyModule.cpp - clang-tidy ----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
+#include "AvoidDoWhileCheck.h"
 #include "AvoidGotoCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"
 #include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"

 namespace cppcoreguidelines {
 
 /// A module containing checks of the C++ Core Guidelines
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
         "cppcoreguidelines-avoid-c-arrays");
+    CheckFactories.registerCheck<AvoidDoWhileCheck>(
+        "cppcoreguidelines-avoid-do-while");
     CheckFactories.registerCheck<AvoidGotoCheck>(
         "cppcoreguidelines-avoid-goto");
     CheckFactories.registerCheck<readability::MagicNumbersCheck>(
         "cppcoreguidelines-avoid-magic-numbers");
     CheckFactories.registerCheck<AvoidNonConstGlobalVariablesCheck>(
         "cppcoreguidelines-avoid-non-const-global-variables");
     CheckFactories.registerCheck<modernize::UseOverrideCheck>(
         "cppcoreguidelines-explicit-virtual-functions");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`altera-id-dependent-backward-branch
   degradation.
 
 - New :doc:`altera-unroll-loops
   <clang-tidy/checks/altera-unroll-loops>` check.
 
   Finds inner loops that have not been unrolled, as well as fully unrolled
   loops with unknown loops bounds or a large number of iterations.
 
+- New :doc:`cppcoreguidelines-avoid-do-while
+  <clang-tidy/checks/cppcoreguidelines-avoid-do-while>` check.
+
+  Checks if a ``do-while`` loop exists and flags it.
+
 - New :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
 
   Finds member initializations in the constructor body which can be placed into
   the initialization list instead.
 
 - New :doc:`bugprone-unhandled-exception-at-new
   <clang-tidy/checks/bugprone-unhandled-exception-at-new>` check.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst

+.. title:: clang-tidy - cppcoreguidelines-avoid-do-while
+
+cppcoreguidelines-avoid-do-while
+================================
+
+Checks if a ``do-while`` loop exists and flags it.
+
+Using a ``while`` loop instead of a ``do-while`` could improve readability and
+prevents overlooking the condition at the end.
+
+Usages of ``do-while`` loops inside a macro definition are not flagged by this
+check if the condition is either ``false``, ``0``, ``nullptr`` or ``__null``.
+
+.. code-block:: c++
+
+  void func1() {
+    int I{0};
+    const int Limit{10};
+
+    // Consider using a while loop
+    do {
+      I++;
+    } while (I <= Limit);
+  }
+
+  void func2() {
+    int I{0};
+    const int Limit{10};
+
+    // Better
+    while (I <= Limit) {
+      I++;
+    }
+  }
+
+This check implements rule `ES.75 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es75-avoid-do-statements>`_ of the C++ Core Guidelines.

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

.. csv-table::
    `clang-analyzer-osx.cocoa.MissingSuperCall <clang-analyzer-osx.cocoa.MissingSuperCall.html>`_,
    `clang-analyzer-osx.cocoa.NonNilReturnValue <clang-analyzer-osx.cocoa.NonNilReturnValue.html>`_,
    `clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak <clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak.html>`_,
    `clang-analyzer-valist.CopyToSelf <clang-analyzer-valist.CopyToSelf.html>`_,
    `clang-analyzer-valist.Uninitialized <clang-analyzer-valist.Uninitialized.html>`_,
    `clang-analyzer-valist.Unterminated <clang-analyzer-valist.Unterminated.html>`_,
    `concurrency-mt-unsafe <concurrency-mt-unsafe.html>`_,
    `concurrency-thread-canceltype-asynchronous <concurrency-thread-canceltype-asynchronous.html>`_,
+   `cppcoreguidelines-avoid-do-while <cppcoreguidelines-avoid-do-while.html>`_,
    `cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
    `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_,
    `cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes"
    `cppcoreguidelines-interfaces-global-init <cppcoreguidelines-interfaces-global-init.html>`_,
    `cppcoreguidelines-macro-usage <cppcoreguidelines-macro-usage.html>`_,
    `cppcoreguidelines-narrowing-conversions <cppcoreguidelines-narrowing-conversions.html>`_,
    `cppcoreguidelines-no-malloc <cppcoreguidelines-no-malloc.html>`_,
    `cppcoreguidelines-owning-memory <cppcoreguidelines-owning-memory.html>`_,

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp

+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-do-while %t
+
+void func1() {
+  int I{0};
+  const int Limit{10};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  do {
+    I++;
+  } while (I <= Limit);
+}
+
+void func2() {
+  int I{0};
+  const int Limit{10};
+
+  // OK
+  while (I <= Limit) {
+    I++;
+  }
+}
+
+void func3() {
+#define MACRO \
+  do {        \
+  } while (true)
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  MACRO;
+
+#undef MACRO
+}
+
+void func4() {
+#define MACRO \
+  do {        \
+  } while (1)
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  MACRO;
+
+#undef MACRO
+}
+
+void func5() {
+#define MACRO \
+  do {        \
+  } while (false)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func6() {
+#define MACRO \
+  do {        \
+  } while (0)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func7() {
+#define MACRO \
+  do {        \
+  } while (nullptr)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func8() {
+#define MACRO \
+  do {        \
+  } while (__null)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}