https://github.com/flowerhack updated
https://github.com/llvm/llvm-project/pull/157213
>From 84a477b27da2e833b6c4da2db7b2a10f00df8b6a Mon Sep 17 00:00:00 2001
From: Julia Hansbrough <flowerh...@google.com>
Date: Fri, 5 Sep 2025 21:27:14 +0000
Subject: [PATCH] Add bugprone-loop-variable-copied-then-modified clang-tidy
check.
Adds a clang-tidy check that alerts when a loop variable is copied and
subsequently modified.
This is a bugprone pattern because the programmer in this case often
assumes they are modifying the original value instead of a copy.
This warning can be suppressed by either converting the loop variable to
a const ref, or by performing the copy explicitly inside the body of the
loop.
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../LoopVariableCopiedThenModifiedCheck.cpp | 100 ++++++++++++++++++
.../LoopVariableCopiedThenModifiedCheck.h | 39 +++++++
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++
.../loop-variable-copied-then-modified.rst | 47 ++++++++
.../docs/clang-tidy/checks/list.rst | 3 +-
...opied-then-modified-ignore-inexpensive.cpp | 55 ++++++++++
.../loop-variable-copied-then-modified.cpp | 50 +++++++++
9 files changed, 305 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/bugprone/loop-variable-copied-then-modified.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified-ignore-inexpensive.cpp
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 824ebdfbd00dc..3ce32d88ea005 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -40,6 +40,7 @@
#include "IntegerDivisionCheck.h"
#include "InvalidEnumDefaultInitializationCheck.h"
#include "LambdaFunctionNameCheck.h"
+#include "LoopVariableCopiedThenModifiedCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
#include "MisleadingSetterOfReferenceCheck.h"
@@ -153,6 +154,8 @@ class BugproneModule : public ClangTidyModule {
"bugprone-incorrect-enable-if");
CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(
"bugprone-incorrect-enable-shared-from-this");
+ CheckFactories.registerCheck<LoopVariableCopiedThenModifiedCheck>(
+ "bugprone-loop-variable-copied-then-modified");
CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>(
"bugprone-unintended-char-ostream-output");
CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 59928e5e47a09..fb28f075b991e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -31,6 +31,7 @@ add_clang_library(clangTidyBugproneModule STATIC
IncorrectEnableIfCheck.cpp
IncorrectEnableSharedFromThisCheck.cpp
InvalidEnumDefaultInitializationCheck.cpp
+ LoopVariableCopiedThenModifiedCheck.cpp
UnintendedCharOstreamOutputCheck.cpp
ReturnConstRefFromParameterCheck.cpp
SuspiciousStringviewDataUsageCheck.cpp
diff --git
a/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.cpp
new file mode 100644
index 0000000000000..4a2bf4b5f85a7
--- /dev/null
+++
b/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.cpp
@@ -0,0 +1,100 @@
+
+//===----------------------------------------------------------------------===//
+//
+// 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 "LoopVariableCopiedThenModifiedCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/TypeTraits.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Basic/Diagnostic.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+LoopVariableCopiedThenModifiedCheck::LoopVariableCopiedThenModifiedCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoreInexpensiveVariables(
+ Options.get("IgnoreInexpensiveVariables", false)) {}
+
+void LoopVariableCopiedThenModifiedCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoreInexpensiveVariables",
IgnoreInexpensiveVariables);
+}
+
+void LoopVariableCopiedThenModifiedCheck::registerMatchers(
+ MatchFinder *Finder) {
+ auto HasReferenceOrPointerTypeOrIsAllowed = hasType(qualType(
+ unless(hasCanonicalType(anyOf(referenceType(), pointerType())))));
+ auto IteratorReturnsValueType = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("*"),
+ callee(
+ cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()))))));
+ auto NotConstructedByCopy = cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor()))));
+ auto ConstructedByConversion =
cxxMemberCallExpr(callee(cxxConversionDecl()));
+ auto LoopVar =
+ varDecl(HasReferenceOrPointerTypeOrIsAllowed,
+ unless(hasInitializer(expr(hasDescendant(expr(
+ anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+ NotConstructedByCopy, ConstructedByConversion)))))));
+ Finder->addMatcher(
+ traverse(TK_AsIs,
+ cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
+ .bind("forRange")),
+ this);
+}
+
+void LoopVariableCopiedThenModifiedCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Var = Result.Nodes.getNodeAs<VarDecl>("loopVar");
+ if (Var->getBeginLoc().isMacroID())
+ return;
+ std::optional<bool> Expensive =
+ utils::type_traits::isExpensiveToCopy(Var->getType(), *Result.Context);
+ if ((!Expensive || !*Expensive) && IgnoreInexpensiveVariables)
+ return;
+ const auto *ForRange = Result.Nodes.getNodeAs<CXXForRangeStmt>("forRange");
+ if (copiedLoopVarIsMutated(*Var, *ForRange, *Result.Context))
+ return;
+}
+
+bool LoopVariableCopiedThenModifiedCheck::copiedLoopVarIsMutated(
+ const VarDecl &LoopVar, const CXXForRangeStmt &ForRange,
+ ASTContext &Context) {
+
+ std::string hintstring = "";
+
+ if (ExprMutationAnalyzer(*ForRange.getBody(), Context).isMutated(&LoopVar)) {
+ if (isa<AutoType>(LoopVar.getType())) {
+ hintstring = "const auto&";
+ } else {
+ std::string CanonicalTypeStr =
+ LoopVar.getType().getAsString(Context.getLangOpts());
+ hintstring = "const " + CanonicalTypeStr + "&";
+ }
+ clang::SourceRange loopvar_source_range =
+ LoopVar.getTypeSourceInfo()->getTypeLoc().getSourceRange();
+ auto Diag =
+ diag(LoopVar.getLocation(), "loop variable '%0' is copied and then "
+ "modified, which is likely a bug; you "
+ "probably want to modify the underlying "
+ "object and not this copy. If you "
+ "*did* intend to modify this copy, "
+ "please use an explicit copy inside the "
+ "body of the loop")
+ << LoopVar.getName()
+ << FixItHint::CreateReplacement(loopvar_source_range, hintstring);
+ return true;
+ }
+ return false;
+}
+
+} // namespace clang::tidy::bugprone
diff --git
a/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.h
b/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.h
new file mode 100644
index 0000000000000..60e44fc50c039
--- /dev/null
+++
b/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.h
@@ -0,0 +1,39 @@
+
+//===----------------------------------------------------------------------===//
+//
+// 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_BUGPRONE_LOOPVARIABLECOPIEDTHENMODIFIEDCHECK_H
+#define
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LOOPVARIABLECOPIEDTHENMODIFIEDCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds loop variables that are copied and subsequently modified.
+///
+/// For the user-facing documentation see:
+///
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/loop-variable-copied-then-modified.html
+class LoopVariableCopiedThenModifiedCheck : public ClangTidyCheck {
+public:
+ LoopVariableCopiedThenModifiedCheck(StringRef Name,
+ ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ 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;
+ }
+ bool copiedLoopVarIsMutated(const VarDecl &LoopVar,
+ const CXXForRangeStmt &ForRange,
+ ASTContext &Context);
+ const bool IgnoreInexpensiveVariables;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif //
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LOOPVARIABLECOPIEDTHENMODIFIEDCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 0f230b8fbdebd..bc95f18c04e84 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -139,6 +139,14 @@ New checks
Detects default initialization (to 0) of variables with ``enum`` type where
the enum has no enumerator with value of 0.
+- New :doc:`bugprone-loop-variable-copied-then-modified
+ <clang-tidy/checks/bugprone/loop-variable-copied-then-modified>` check.
+
+ Detects when a loop variable is copied and then subsequently modified.
+ Suggests replacing such instances with either a const ref (to prevent the
+ copy) or by performing the copy explicitly inside the loop (to make it
+ obvious one intends to modify a copy instead of the underlying object).
+
- New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access
<clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>`
check.
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/loop-variable-copied-then-modified.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/loop-variable-copied-then-modified.rst
new file mode 100644
index 0000000000000..f2e2f2dc5f630
--- /dev/null
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/loop-variable-copied-then-modified.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-loop-variable-copied-then-modified
+
+bugprone-loop-variable-copied-then-modified
+===========================================
+
+Warns when a loop variable is copied and subsequently modified.
+
+This pattern is considered bugprone because, frequently, programmers do not
+realize that they are modifying a *copy* rather than an underlying value,
+resulting in subtly erroneous code.
+
+For instance, the following code attempts to null out a value in a map, but
only
+succeeds in
+
+.. code-block:: c++
+
+ for (auto target : target_map) {
+ target.value = nullptr;
+ }
+
+The programmer is likely to have intended this code instead:
+
+.. code-block:: c++
+
+ for (const auto& target : target_map) {
+ target.value = nullptr;
+ }
+
+This warning can be suppressed in one of two ways:
+ - In cases where the programmer did not intend to create a copy, they can
+ convert the loop variable to a const reference. A FixIt message will
+ provide a naive suggestion of how to achieve this, which works in most
+ cases.
+ - In cases where the intent is in fact to modify a copy, they may perform the
+ copy inside the body of the loop, and perform whatever operations they like
+ on that copy.
+
+This is a conservative check: in cases where it cannot be determined at compile
+time whether or not a particular function modifies the variable, it assumes a
+modification has ocurred and warns accordingly. However, in such cases, the
+warning will still be suppressed by doing one of the actions described above.
+
+.. option:: IgnoreInexpensiveVariables
+
+ When `true`, this check will only alert on types that are expensive to copy.
+ This will lead to fewer "false" positives, but will also overlook some
+ instances where there may be an actual bug.
\ No newline at end of file
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5e3ffc4f8aca3..bb88a4d3e05b1 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -108,6 +108,7 @@ Clang-Tidy Checks
:doc:`bugprone-integer-division <bugprone/integer-division>`,
:doc:`bugprone-invalid-enum-default-initialization
<bugprone/invalid-enum-default-initialization>`,
:doc:`bugprone-lambda-function-name <bugprone/lambda-function-name>`,
+ :doc:`bugprone-loop-variable-copied-then-modified
<bugprone/loop-variable-copied-then-modified>`, "Yes"
:doc:`bugprone-macro-parentheses <bugprone/macro-parentheses>`, "Yes"
:doc:`bugprone-macro-repeated-side-effects
<bugprone/macro-repeated-side-effects>`,
:doc:`bugprone-misleading-setter-of-reference
<bugprone/misleading-setter-of-reference>`,
@@ -249,12 +250,12 @@ Clang-Tidy Checks
:doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`,
:doc:`llvm-header-guard <llvm/header-guard>`,
:doc:`llvm-include-order <llvm/include-order>`, "Yes"
- :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
:doc:`llvm-namespace-comment <llvm/namespace-comment>`,
:doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals
<llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes"
:doc:`llvm-prefer-register-over-unsigned
<llvm/prefer-register-over-unsigned>`, "Yes"
:doc:`llvm-prefer-static-over-anonymous-namespace
<llvm/prefer-static-over-anonymous-namespace>`,
:doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
+ :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
:doc:`llvmlibc-callee-namespace <llvmlibc/callee-namespace>`,
:doc:`llvmlibc-implementation-in-namespace
<llvmlibc/implementation-in-namespace>`,
:doc:`llvmlibc-inline-function-decl <llvmlibc/inline-function-decl>`, "Yes"
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified-ignore-inexpensive.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified-ignore-inexpensive.cpp
new file mode 100644
index 0000000000000..7e14b3dcea7da
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified-ignore-inexpensive.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s bugprone-loop-variable-copied-then-modified %t --
-- -I%S -std=c++!4 -config="{CheckOptions:
{bugprone-loop-variable-copied-then-modified.IgnoreInexpensiveVariables: true}}"
+
+#include "Inputs/system-header-simulator/sim_set"
+#include "Inputs/system-header-simulator/sim_unordered_set"
+#include "Inputs/system-header-simulator/sim_map"
+#include "Inputs/system-header-simulator/sim_unordered_map"
+#include "Inputs/system-header-simulator/sim_vector"
+#include "Inputs/system-header-simulator/sim_algorithm"
+
+template <typename T>
+struct Iterator {
+ void operator++() {}
+ const T& operator*() {
+ static T* TT = new T();
+ return *TT;
+ }
+ bool operator!=(const Iterator &) { return false; }
+};
+template <typename T>
+struct View {
+ T begin() { return T(); }
+ T begin() const { return T(); }
+ T end() { return T(); }
+ T end() const { return T(); }
+};
+
+struct S {
+ int value;
+
+ S() : value(0) {};
+ S(const S &);
+ ~S();
+ S &operator=(const S &);
+ void modify() {
+ value++;
+ }
+};
+
+void NegativeOnlyCopyingInts() {
+ std::vector<int> foo;
+ foo.push_back(1);
+ foo.push_back(2);
+ foo.push_back(3);
+ for (int v : foo) {
+ v += 1;
+ }
+}
+
+void PositiveLoopVariableCopiedAndThenModfied() {
+ for (S S1 : View<Iterator<S>>()) {
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: loop variable 'S1' is copied
and then modified, which is likely a bug; you probably want to modify the
underlying object and not this copy. If you *did* intend to modify this copy,
please use an explicit copy inside the body of the loop
+ // CHECK-FIXES: for (const S& S1 : View<Iterator<S>>()) {
+ S1.modify();
+ }
+}
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified.cpp
new file mode 100644
index 0000000000000..924738f14a86a
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s bugprone-loop-variable-copied-then-modified %t
+
+template <typename T>
+struct Iterator {
+ void operator++() {}
+ const T& operator*() {
+ static T* TT = new T();
+ return *TT;
+ }
+ bool operator!=(const Iterator &) { return false; }
+};
+template <typename T>
+struct View {
+ T begin() { return T(); }
+ T begin() const { return T(); }
+ T end() { return T(); }
+ T end() const { return T(); }
+};
+
+struct S {
+ int value;
+
+ S() : value(0) {};
+ S(const S &);
+ ~S();
+ S &operator=(const S &);
+ void modify() {
+ value++;
+ }
+};
+
+void NegativeLoopVariableNotCopied() {
+ for (const S& S1 : View<Iterator<S>>()) {
+ // It's fine to copy-by-value S1 into some other S.
+ S S2 = S1;
+ }
+}
+
+void NegativeLoopVariableCopiedButNotModified() {
+ for (S S1 : View<Iterator<S>>()) {
+ }
+}
+
+void PositiveLoopVariableCopiedAndThenModfied() {
+ for (S S1 : View<Iterator<S>>()) {
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: loop variable 'S1' is copied
and then modified, which is likely a bug; you probably want to modify the
underlying object and not this copy. If you *did* intend to modify this copy,
please use an explicit copy inside the body of the loop
+ // CHECK-FIXES: for (const S& S1 : View<Iterator<S>>()) {
+ S1.modify();
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
