DNS320 updated this revision to Diff 341784.
DNS320 added a comment.
Renamed the IsInsideMatchedForStmt() function according to a comment from the
build system.
Changed a data type to "auto" according to a comment from Eugene.Zelenko.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100092/new/
https://reviews.llvm.org/D100092
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-declare-loop-variable-in-the-initializer %t
+
+const int Limit{10};
+
+void func1() {
+ // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+ // CHECK-MESSAGES: :11:5: note: Variable gets modified here
+ int A{0};
+
+ for (int I{0}; I < Limit; I++) {
+ A = 3;
+ }
+}
+
+void func2() {
+ // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+ // CHECK-MESSAGES: :21:5: note: Variable gets modified here
+ int A{0};
+
+ for (int I{0}; I < Limit; I++) {
+ A += 3;
+ }
+}
+
+void func3() {
+ // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+ // CHECK-MESSAGES: :30:8: note: Variable gets modified here
+ int I{0};
+
+ for (I = 1; I < Limit; I++) {
+ }
+}
+
+void func4() {
+ // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+ // CHECK-MESSAGES: :39:21: note: Variable gets modified here
+ int I{0};
+
+ for (; I < Limit; I++) {
+ }
+}
+
+void func5() {
+ // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+ // CHECK-MESSAGES: :48:34: note: Variable gets modified here
+ int I{0};
+
+ for (int Unused{0}; I < Limit; I++) {
+ }
+}
+
+void func6() {
+ // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+ // CHECK-MESSAGES: :58:8: note: Variable gets modified here
+ int I{0};
+ int Z{0};
+
+ for (I = 3; I < Limit; I++) {
+ }
+ Z = 3;
+}
+
+void func7() {
+ // OK
+ int A{0};
+
+ for (int I{0}; I < Limit; I++) {
+ const int B{A};
+ }
+}
+
+void func8() {
+ // OK
+ int I{0};
+
+ for (I = 0; I < Limit; I++) {
+ }
+ const int A{I};
+}
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
@@ -143,6 +143,7 @@
`concurrency-thread-canceltype-asynchronous <concurrency-thread-canceltype-asynchronous.html>`_,
`cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
`cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_,
+ `cppcoreguidelines-declare-loop-variable-in-the-initializer <cppcoreguidelines-declare-loop-variable-in-the-initializer.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>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - cppcoreguidelines-declare-loop-variable-in-the-initializer
+
+cppcoreguidelines-declare-loop-variable-in-the-initializer
+==========================================================
+
+Finds variables that are modified inside a ``for`` statement, are not used elsewhere
+and are declared outside the ``for`` statement.
+
+.. code-block:: c++
+
+ const int Limit{10};
+
+ void func1() {
+ // Should print a warning
+ int A{0};
+
+ for(int I{0}; I < Limit; I++) {
+ A = 3;
+ }
+ }
+
+ void func2() {
+ // Should print a warning
+ int I{0};
+
+ for(I = 1; I < Limit; I++) {}
+ }
+
+ void func3() {
+ // Should print a warning
+ int I{0};
+
+ for(int Unused{0}; I < Limit; I++) {}
+ }
+
+ void func4() {
+ // OK
+ int A{0};
+
+ for(int I{0}; I < Limit; I++) {
+ const int B{A};
+ }
+ }
+
+This check implements the rule `ES.74 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es74-prefer-to-declare-a-loop-variable-in-the-initializer-part-of-a-for-statement>`_ of the C++ Core Guidelines.
+
+It does also cover parts of:
+ - `ES.26 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es26-dont-use-a-variable-for-two-unrelated-purposes>`_
+ - `ES.5 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es5-keep-scopes-small>`_
+ - `ES.6 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es6-declare-names-in-for-statement-initializers-and-conditions-to-limit-scope>`_
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -89,6 +89,12 @@
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-declare-loop-variable-in-the-initializer
+ <clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer>` check.
+
+ Finds variables that are modified inside a ``for`` statement, are not used elsewhere
+ and are declared outside the ``for`` statement.
+
- New :doc:`cppcoreguidelines-prefer-member-initializer
<clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
@@ -0,0 +1,39 @@
+//===--- DeclareLoopVariableInTheInitializerCheck.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_DECLARELOOPVARIABLEINTHEINITIALIZERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_DECLARELOOPVARIABLEINTHEINITIALIZERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Finds variables that are modified inside a ``for`` statement, are not used
+/// elsewhere and are declared outside the ``for`` statement.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.html
+class DeclareLoopVariableInTheInitializerCheck : public ClangTidyCheck {
+public:
+ DeclareLoopVariableInTheInitializerCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_DECLARELOOPVARIABLEINTHEINITIALIZERCHECK_H
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
@@ -0,0 +1,104 @@
+//===--- DeclareLoopVariableInTheInitializerCheck.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 "DeclareLoopVariableInTheInitializerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+class OutsideForStmtVisitor
+ : public RecursiveASTVisitor<OutsideForStmtVisitor> {
+ friend class RecursiveASTVisitor<OutsideForStmtVisitor>;
+
+public:
+ OutsideForStmtVisitor(const ForStmt *ForStmt, const VarDecl *VarDecl,
+ const MatchFinder::MatchResult &Result)
+ : MatchedForStmt(ForStmt), MatchedDecl(VarDecl), MatchedResult(Result),
+ IsOutsideMatchedForStmt(false) {}
+
+ bool hasDeclRefExpOutside(const CompoundStmt *Compound) {
+ TraverseStmt(const_cast<CompoundStmt *>(Compound));
+ return IsOutsideMatchedForStmt;
+ }
+
+private:
+ bool VisitDeclRefExpr(DeclRefExpr *D) {
+ if (const auto *To = dyn_cast<VarDecl>(D->getDecl())) {
+ if (To == MatchedDecl &&
+ !isInsideMatchedForStmt(MatchedResult, DynTypedNode::create(*D))) {
+ IsOutsideMatchedForStmt = true;
+ return false;
+ }
+ }
+ return true;
+ }
+
+ bool isInsideMatchedForStmt(const MatchFinder::MatchResult &Result,
+ const DynTypedNode &Node) {
+ const auto *PossibleForStmt = Node.get<ForStmt>();
+
+ if (PossibleForStmt && PossibleForStmt == MatchedForStmt) {
+ return true;
+ }
+
+ return llvm::any_of(Result.Context->getParents(Node),
+ [&](const DynTypedNode &Parent) {
+ return isInsideMatchedForStmt(Result, Parent);
+ });
+ }
+
+ const ForStmt *MatchedForStmt;
+ const VarDecl *MatchedDecl;
+ const MatchFinder::MatchResult &MatchedResult;
+ bool IsOutsideMatchedForStmt;
+};
+
+void DeclareLoopVariableInTheInitializerCheck::registerMatchers(
+ MatchFinder *Finder) {
+ Finder->addMatcher(
+ declRefExpr(
+ hasAncestor(forStmt().bind("ForStmt")),
+ anyOf(hasAncestor(unaryOperator().bind("Operator")),
+ hasAncestor(
+ binaryOperator(isAssignmentOperator()).bind("Operator"))),
+ to(varDecl(hasAncestor(compoundStmt().bind("Compound")),
+ unless(hasAncestor(forStmt(equalsBoundNode("ForStmt")))))
+ .bind("VarDecl"))),
+ this);
+}
+
+void DeclareLoopVariableInTheInitializerCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedForStmt = Result.Nodes.getNodeAs<ForStmt>("ForStmt");
+ const auto *MatchedExprOperator = Result.Nodes.getNodeAs<Expr>("Operator");
+ const auto *MatchedVarDecl = Result.Nodes.getNodeAs<VarDecl>("VarDecl");
+ const auto *MatchedCompoundStmt =
+ Result.Nodes.getNodeAs<CompoundStmt>("Compound");
+
+ if (OutsideForStmtVisitor(MatchedForStmt, MatchedVarDecl, Result)
+ .hasDeclRefExpOutside(MatchedCompoundStmt)) {
+ return;
+ }
+
+ diag(MatchedVarDecl->getLocation(),
+ "Variable %0 is only modified in a for statement and not used "
+ "elsewhere. Consider declaring it inside the for statement.")
+ << MatchedVarDecl;
+ diag(MatchedExprOperator->getBeginLoc(), "Variable gets modified here",
+ DiagnosticIDs::Note);
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
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
@@ -16,6 +16,7 @@
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
#include "AvoidNonConstGlobalVariablesCheck.h"
+#include "DeclareLoopVariableInTheInitializerCheck.h"
#include "InitVariablesCheck.h"
#include "InterfacesGlobalInitCheck.h"
#include "MacroUsageCheck.h"
@@ -52,6 +53,8 @@
"cppcoreguidelines-avoid-magic-numbers");
CheckFactories.registerCheck<AvoidNonConstGlobalVariablesCheck>(
"cppcoreguidelines-avoid-non-const-global-variables");
+ CheckFactories.registerCheck<DeclareLoopVariableInTheInitializerCheck>(
+ "cppcoreguidelines-declare-loop-variable-in-the-initializer");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"cppcoreguidelines-explicit-virtual-functions");
CheckFactories.registerCheck<InitVariablesCheck>(
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
@@ -7,6 +7,7 @@
AvoidGotoCheck.cpp
AvoidNonConstGlobalVariablesCheck.cpp
CppCoreGuidelinesTidyModule.cpp
+ DeclareLoopVariableInTheInitializerCheck.cpp
InitVariablesCheck.cpp
InterfacesGlobalInitCheck.cpp
MacroUsageCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
