DNS320 updated this revision to Diff 346360.
DNS320 added a comment.

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 
<https://clang.llvm.org/extra/clang-tidy/checks/bugprone-terminating-continue.html>)

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102576/new/

https://reviews.llvm.org/D102576

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
@@ -0,0 +1,87 @@
+// 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
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -144,6 +144,7 @@
    `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"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
@@ -0,0 +1,36 @@
+.. 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.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   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.
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -14,6 +14,7 @@
 #include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
+#include "AvoidDoWhileCheck.h"
 #include "AvoidGotoCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
@@ -46,6 +47,8 @@
   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>(
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
+  AvoidDoWhileCheck.cpp
   AvoidGotoCheck.cpp
   AvoidNonConstGlobalVariablesCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
@@ -0,0 +1,37 @@
+//===--- 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
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
@@ -0,0 +1,42 @@
+//===--- 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to