llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Julia Hansbrough (flowerhack) <details> <summary>Changes</summary> See documentation for purpose and further details. --- Full diff: https://github.com/llvm/llvm-project/pull/152970.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3) - (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1) - (added) clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.cpp (+44) - (added) clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.h (+34) - (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-copy-in-range-loop.rst (+31) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-copy-in-range-loop.cpp (+7) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 824ebdfbd00dc..1e08d9b0ec977 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/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>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 59928e5e47a09..c1835b5e4f82b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.cpp new file mode 100644 index 0000000000000..9f93a6bc2aeb0 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousCopyInRangeLoopCheck.h new file mode 100644 index 0000000000000..fe8c7bad76ae0 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-copy-in-range-loop.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-copy-in-range-loop.rst new file mode 100644 index 0000000000000..6add3192fb7fa --- /dev/null +++ b/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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-copy-in-range-loop.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-copy-in-range-loop.cpp new file mode 100644 index 0000000000000..3316c28e06f88 --- /dev/null +++ b/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 `````````` </details> https://github.com/llvm/llvm-project/pull/152970 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
