https://github.com/flowerhack updated
https://github.com/llvm/llvm-project/pull/157213
>From ce57f8591dad2dae6812cd783c396b11da968152 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.
Fix GH-155922
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../LoopVariableCopiedThenModifiedCheck.cpp | 89 +++++++++++++++++++
.../LoopVariableCopiedThenModifiedCheck.h | 35 ++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++
.../loop-variable-copied-then-modified.rst | 48 ++++++++++
.../docs/clang-tidy/checks/list.rst | 1 +
...opied-then-modified-ignore-inexpensive.cpp | 50 +++++++++++
.../loop-variable-copied-then-modified.cpp | 79 ++++++++++++++++
9 files changed, 312 insertions(+)
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 fe261e729539c..e5b55ce1c5283 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"
@@ -154,6 +155,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 46bc8efd44bc5..52e81fed9f7cc 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..7a5f30db8d2e4
--- /dev/null
+++
b/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.cpp
@@ -0,0 +1,89 @@
+//===----------------------------------------------------------------------===//
+//
+// 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) {
+ const auto HasReferenceOrPointerTypeOrIsAllowed = hasType(qualType(
+ unless(hasCanonicalType(anyOf(referenceType(), pointerType())))));
+ const auto IteratorReturnsValueType = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("*"),
+ callee(
+ cxxMethodDecl(returns(unless(hasCanonicalType(referenceType()))))));
+ const auto NotConstructedByCopy = cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(unless(isCopyConstructor()))));
+ const auto ConstructedByConversion =
+ cxxMemberCallExpr(callee(cxxConversionDecl()));
+ const auto LoopVar =
+ varDecl(HasReferenceOrPointerTypeOrIsAllowed,
+ unless(hasInitializer(expr(hasDescendant(expr(
+ anyOf(materializeTemporaryExpr(), IteratorReturnsValueType,
+ NotConstructedByCopy, ConstructedByConversion)))))));
+ Finder->addMatcher(cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
+ .bind("forRange"),
+ this);
+}
+
+void LoopVariableCopiedThenModifiedCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *LoopVar = Result.Nodes.getNodeAs<VarDecl>("loopVar");
+ if (LoopVar->getBeginLoc().isMacroID())
+ return;
+ std::optional<bool> Expensive = utils::type_traits::isExpensiveToCopy(
+ LoopVar->getType(), *Result.Context);
+ if ((!Expensive || !*Expensive) && IgnoreInexpensiveVariables)
+ return;
+ const auto *ForRange = Result.Nodes.getNodeAs<CXXForRangeStmt>("forRange");
+
+ std::string HintString = "";
+
+ if (ExprMutationAnalyzer(*ForRange->getBody(), *Result.Context)
+ .isMutated(LoopVar)) {
+ if (isa<AutoType>(LoopVar->getType())) {
+ HintString = "const auto&";
+ } else {
+ const std::string CanonicalTypeStr =
+ LoopVar->getType().getAsString(Result.Context->getLangOpts());
+ HintString = "const " + CanonicalTypeStr + "&";
+ }
+ clang::SourceRange LoopVarSourceRange =
+ LoopVar->getTypeSourceInfo()->getTypeLoc().getSourceRange();
+ 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(LoopVarSourceRange, HintString);
+ }
+}
+
+} // 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..14a4011703c2d
--- /dev/null
+++
b/clang-tools-extra/clang-tidy/bugprone/LoopVariableCopiedThenModifiedCheck.h
@@ -0,0 +1,35 @@
+//===----------------------------------------------------------------------===//
+//
+// 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;
+ }
+ 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 23d757b5e6f2e..0f43bb4197f7e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -148,6 +148,12 @@ 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 and
+ suggests replacing with a ``const`` reference or an explicit copy.
+
- 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..809d9caeb2457
--- /dev/null
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/loop-variable-copied-then-modified.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-loop-variable-copied-then-modified
+
+bugprone-loop-variable-copied-then-modified
+===========================================
+
+Detects when a loop variable is copied and then subsequently modified and
+suggests replacing with a reference or an explicit copy.
+
+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 nulling out a value in a *copy* of the map:
+
+.. 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 (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 reference or a ``const`` reference. A fix-it
+ 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 explicitly, 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 can 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. Default is `false`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c490d2ece2e0a..3ee2cae65accb 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>`,
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..6781d8d691c58
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified-ignore-inexpensive.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s bugprone-loop-variable-copied-then-modified %t --
-config="{CheckOptions:
{bugprone-loop-variable-copied-then-modified.IgnoreInexpensiveVariables:
true}}" -- -I%S -std=c++!4
+#include "Inputs/system-header-simulator/sim_initializer_list"
+#include "Inputs/system-header-simulator/sim_vector"
+
+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();
+ }
+}
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..abecf7f07fc65
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/loop-variable-copied-then-modified.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %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++;
+ }
+};
+
+template <typename V>
+struct Generic {
+ V value;
+
+ Generic() : value{} {};
+ Generic(const Generic &);
+ ~Generic();
+ Generic &operator=(const Generic &);
+ 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();
+ }
+}
+
+void PositiveLoopVariableCopiedAndThenModifiedAuto() {
+ for (auto S1 : View<Iterator<S>>()) {
+ // CHECK-MESSAGES: [[@LINE-1]]:13: 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 auto& S1 : View<Iterator<S>>()) {
+ S1.modify();
+ }
+}
+
+void PositiveLoopVariableCopiedAndThenModfiedGeneric() {
+ for (Generic G : View<Iterator<Generic<double>>>()) {
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: loop variable 'G' 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 Generic<double>& G :
View<Iterator<Generic<double>>>()) {
+ G.modify();
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
