[clang-tidy] Adds a SuspiciousCopyInRangeLoop check.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -73,6 +73,7 @@
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "StringviewNullptrCheck.h"
+#include "SuspiciousCopyInRangeLoopCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
 #include "SuspiciousIncludeCheck.h"
 #include "SuspiciousMemoryComparisonCheck.h"
@@ -153,6 +154,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-incorrect-enable-if");
     CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(
         "bugprone-incorrect-enable-shared-from-this");
+    CheckFactories.registerCheck<SuspiciousCopyInRangeLoopCheck>(
+        "bugprone-suspicious-copy-in-range-loop");
     CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>(
         "bugprone-unintended-char-ostream-output");
     CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(

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

@@ -33,6 +33,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   InvalidEnumDefaultInitializationCheck.cpp
   UnintendedCharOstreamOutputCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
+  SuspiciousCopyInRangeLoopCheck.cpp
   SuspiciousStringviewDataUsageCheck.cpp
   SwitchMissingDefaultCaseCheck.cpp
   IncDecInConditionsCheck.cpp

clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.cpp

@@ -0,0 +1,44 @@
+//===--- suspiciousCopyInRangeLoopCheck.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 "SuspiciousCopyInRangeLoopCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+// TODO: Possibly rename to "SuspiciousAutoCopyInRangeLoop"
+void SuspiciousCopyInRangeLoopCheck::registerMatchers(MatchFinder *Finder) {
+  // TODO: make sure this catches `const auto` as well.
+  auto auto_copy_in_range_based_for_loops =
+      cxxForRangeStmt(hasLoopVariable(varDecl(hasType(autoType()))));
+  Finder->addMatcher(auto_copy_in_range_based_for_loops.bind("for_range"),
+                     this);
+}
+
+void SuspiciousCopyInRangeLoopCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+      Result.Nodes.getNodeAs<CXXForRangeStmt>("for_range");
+  std::string VarName = MatchedDecl->getLoopVariable()->getNameAsString();
+  if (MatchedDecl) {
+    diag(MatchedDecl->getBeginLoc(),
+         "Found potentially-spurious copy in range loop:\n* It is unlikely you "
+         "intended to copy '%0' for each iteration of the loop\n* To avoid "
+         "copying, use `auto&`\n* If this copy was intentional, do not use "
+         "`auto` and instead spell out the type explicitly")
+        << VarName
+        << FixItHint::CreateInsertion(
+               MatchedDecl->getLoopVariable()->getLocation(),
+               "suspicious copy here");
+  }
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.h

@@ -0,0 +1,34 @@
+//===--- SuspiciousCopyInRangeLoopCheck.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_BUGPRONE_SUSPICIOUSCOPYINRANGELOOPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSCOPYINRANGELOOPCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/supsicious-copy-in-range-loop.html
+class SuspiciousCopyInRangeLoopCheck : public ClangTidyCheck {
+public:
+  SuspiciousCopyInRangeLoopCheck(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 clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSCOPYINRANGELOOPCHECK_H

clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-copy-in-range-loop.rst

@@ -0,0 +1,31 @@
+.. title:: clang-tidy - bugprone-suspicious-copy-in-range-loop
+
+bugprone-suspicious-copy-in-range-loop
+================================
+
+This check finds ``for`` loops that are potentially-unintentionally copying
+the loop variable unnecessarily.
+
+For instance, this check would warn on a loop of the form:
+
+```for (auto s : strings) {
+    ...
+}
+```
+
+This can lead to bugs when the programmer operates on `s` in the loop, assuming
+they are mutating the elements directly, but they are in fact only mutating a
+temporary copy.
+
+In cases where the programmer's intention is to in fact copy the variable,
+the warning can be suppressed by explicitly stating the type.
+For instance, this check will *not* warn on a loop of the form
+
+```for (std::string s : strings) {
+ ...
+ }
+ ```
+
+This check is different from `performance-for-range-copy` in that
+it triggers on *every* instance where this pattern occurs rather than
+potentially-expensive ones.

clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-copy-in-range-loop.cpp

@@ -0,0 +1,7 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-copy-in-range-loop %t
+
+std::vector<std::string> some_strings;
+for (auto x : some_strings) {
+	std::cout << x << "\n";
+}
+// CHECK-MESSAGES: foo