[clang-tools-extra] Create a new check to look for mis-use in calls that take iterators (PR #99917)

Nathan James via cfe-commits cfe-commits at lists.llvm.org
Sat Jul 27 11:12:07 PDT 2024


https://github.com/njames93 updated https://github.com/llvm/llvm-project/pull/99917

>From c79951fdc4f866413f83a94b1eaa16f3ffb10aa4 Mon Sep 17 00:00:00 2001
From: Nathan James <n.james93 at hotmail.co.uk>
Date: Tue, 23 Jul 2024 10:59:45 +0100
Subject: [PATCH] Create a new check to look for mis-use in calls that take
 iterators

Looks for various patterns of functions that take arguments calling
begin/end or similar for common mistakes
---
 .../bugprone/BugproneTidyModule.cpp           |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt        |   1 +
 .../bugprone/IncorrectIteratorsCheck.cpp      | 838 ++++++++++++++++++
 .../bugprone/IncorrectIteratorsCheck.h        |  45 +
 clang-tools-extra/docs/ReleaseNotes.rst       |   6 +
 .../checks/bugprone/incorrect-iterators.rst   | 131 +++
 .../docs/clang-tidy/checks/list.rst           |   1 +
 .../checkers/bugprone/incorrect-iterators.cpp | 282 ++++++
 8 files changed, 1307 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-iterators.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 689eb92a3d8d1..cea040b81878a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -33,6 +33,7 @@
 #include "InaccurateEraseCheck.h"
 #include "IncDecInConditionsCheck.h"
 #include "IncorrectEnableIfCheck.h"
+#include "IncorrectIteratorsCheck.h"
 #include "IncorrectRoundingsCheck.h"
 #include "InfiniteLoopCheck.h"
 #include "IntegerDivisionCheck.h"
@@ -139,6 +140,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-inaccurate-erase");
     CheckFactories.registerCheck<IncorrectEnableIfCheck>(
         "bugprone-incorrect-enable-if");
+    CheckFactories.registerCheck<IncorrectIteratorsCheck>(
+        "bugprone-incorrect-iterators");
     CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
         "bugprone-return-const-ref-from-parameter");
     CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index cb0d8ae98bac5..8425dbed0505a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectEnableIfCheck.cpp
+  IncorrectIteratorsCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SuspiciousStringviewDataUsageCheck.cpp
   SwitchMissingDefaultCaseCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp
new file mode 100644
index 0000000000000..3054720522756
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp
@@ -0,0 +1,838 @@
+//===--- IncorrectIteratorsCheck.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 "IncorrectIteratorsCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
+#include <functional>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+using SVU = llvm::SmallVector<unsigned, 3>;
+/// Checks to see if a all the parameters of a template function with a given
+/// index refer to the same type.
+AST_MATCHER_P(FunctionDecl, areParametersSameTemplateType, SVU, Indexes) {
+  const FunctionTemplateDecl *TemplateDecl = Node.getPrimaryTemplate();
+  if (!TemplateDecl)
+    return false;
+  const FunctionDecl *FuncDecl = TemplateDecl->getTemplatedDecl();
+  if (!FuncDecl)
+    return false;
+  assert(!Indexes.empty());
+  if (llvm::any_of(Indexes, [Count(FuncDecl->getNumParams())](unsigned Index) {
+        return Index >= Count;
+      }))
+    return false;
+  const ParmVarDecl *FirstParam = FuncDecl->getParamDecl(Indexes.front());
+  if (!FirstParam)
+    return false;
+  QualType Type = FirstParam->getOriginalType();
+  for (auto Item : llvm::drop_begin(Indexes)) {
+    const ParmVarDecl *Param = FuncDecl->getParamDecl(Item);
+    if (!Param)
+      return false;
+    if (Param->getOriginalType() != Type)
+      return false;
+  }
+  return true;
+}
+AST_MATCHER_P(FunctionDecl, isParameterTypeUnique, unsigned, Index) {
+  const FunctionTemplateDecl *TemplateDecl = Node.getPrimaryTemplate();
+  if (!TemplateDecl)
+    return false;
+  const FunctionDecl *FuncDecl = TemplateDecl->getTemplatedDecl();
+  if (!FuncDecl)
+    return false;
+  if (Index >= FuncDecl->getNumParams())
+    return false;
+  const ParmVarDecl *MainParam = FuncDecl->getParamDecl(Index);
+  if (!MainParam)
+    return false;
+  QualType Type = MainParam->getOriginalType();
+  for (unsigned I = 0, E = FuncDecl->getNumParams(); I != E; ++I) {
+    if (I == Index)
+      continue;
+    const ParmVarDecl *Param = FuncDecl->getParamDecl(I);
+    if (!Param)
+      continue;
+    if (Param->getOriginalType() == Type)
+      return false;
+  }
+  return true;
+}
+AST_MATCHER(Expr, isNegativeIntegerConstant) {
+  if (auto Res = Node.getIntegerConstantExpr(Finder->getASTContext())) {
+    return Res->isNegative();
+  }
+  return false;
+}
+struct NameMatchers {
+  ArrayRef<StringRef> FreeNames;
+  ArrayRef<StringRef> MethodNames;
+};
+
+struct NamePairs {
+  NameMatchers BeginNames;
+  NameMatchers EndNames;
+};
+
+struct FullState {
+  NamePairs Forward;
+  NamePairs Reversed;
+  NamePairs Combined;
+  NameMatchers All;
+  ArrayRef<StringRef> MakeReverseIterator;
+};
+
+} // namespace
+
+static constexpr char FirstRangeArg[] = "FirstRangeArg";
+static constexpr char FirstRangeArgExpr[] = "FirstRangeArgExpr";
+static constexpr char ReverseBeginBind[] = "ReverseBeginBind";
+static constexpr char ReverseEndBind[] = "ReverseEndBind";
+static constexpr char SecondRangeArg[] = "SecondRangeArg";
+static constexpr char SecondRangeArgExpr[] = "SecondRangeArgExpr";
+static constexpr char ArgMismatchBegin[] = "ArgMismatchBegin";
+static constexpr char ArgMismatchEnd[] = "ArgMismatchEnd";
+static constexpr char ArgMismatchExpr[] = "ArgMismatchExpr";
+static constexpr char ArgMismatchRevBind[] = "ArgMismatchRevBind";
+static constexpr char Callee[] = "Callee";
+static constexpr char Internal[] = "Internal";
+static constexpr char InternalOther[] = "InternalOther";
+static constexpr char InternalArgument[] = "InternalArgument";
+static constexpr char OutputRangeEnd[] = "OutputRangeEnd";
+static constexpr char OutputRangeBegin[] = "OutputRangeBegin";
+static constexpr char UnexpectedDec[] = "UndexpectedDec";
+static constexpr char UnexpectedInc[] = "UnexpectedInc";
+static constexpr char UnexpectedIncDecExpr[] = "UnexpectedIncDecExpr";
+
+static auto
+makeExprMatcher(ast_matchers::internal::Matcher<Expr> ArgumentMatcher,
+                const NameMatchers &Names, ArrayRef<StringRef> MakeReverse,
+                const NameMatchers &RevNames, StringRef RevBind) {
+  auto MakeMatcher = [&ArgumentMatcher](const NameMatchers &Names) {
+    return anyOf(
+        cxxMemberCallExpr(argumentCountIs(0),
+                          callee(cxxMethodDecl(hasAnyName(Names.MethodNames))),
+                          on(ArgumentMatcher)),
+        callExpr(argumentCountIs(1),
+                 hasDeclaration(functionDecl(hasAnyName(Names.FreeNames))),
+                 hasArgument(0, ArgumentMatcher)));
+  };
+  return expr(anyOf(MakeMatcher(Names),
+                    callExpr(argumentCountIs(1),
+                             callee(functionDecl(hasAnyName(MakeReverse))),
+                             hasArgument(0, MakeMatcher(RevNames)))
+                        .bind(RevBind)));
+}
+
+/// Detects a range function where the argument for the begin call differs from
+/// the end call
+/// @code
+///   std::find(I.begin(), J.end());
+/// @endcode
+static auto makeRangeArgMismatch(unsigned BeginExpected, unsigned EndExpected,
+                                 NamePairs Forward, NamePairs Reverse,
+                                 ArrayRef<StringRef> MakeReverse) {
+  std::vector<ast_matchers::internal::DynTypedMatcher> Matchers;
+
+  for (const auto &[Primary, Backwards] :
+       {std::pair{Forward, Reverse}, std::pair{Reverse, Forward}}) {
+    Matchers.push_back(callExpr(
+        hasArgument(BeginExpected,
+                    makeExprMatcher(expr().bind(FirstRangeArg),
+                                    Primary.BeginNames, MakeReverse,
+                                    Backwards.EndNames, ReverseBeginBind)
+                        .bind(FirstRangeArgExpr)),
+        hasArgument(EndExpected,
+                    makeExprMatcher(
+                        expr(unless(matchers::isStatementIdenticalToBoundNode(
+                                 FirstRangeArg)))
+                            .bind(SecondRangeArg),
+                        Primary.EndNames, MakeReverse, Backwards.BeginNames,
+                        ReverseEndBind)
+                        .bind(SecondRangeArgExpr))));
+  }
+
+  return callExpr(
+      argumentCountAtLeast(std::max(BeginExpected, EndExpected) + 1),
+      ast_matchers::internal::DynTypedMatcher::constructVariadic(
+          ast_matchers::internal::DynTypedMatcher::VO_AnyOf,
+          ASTNodeKind::getFromNodeKind<CallExpr>(), std::move(Matchers))
+          .convertTo<CallExpr>());
+}
+
+/// Detects a range function where we expect a call to begin but get a call to
+/// end or vice versa
+/// @code
+///   std::find(X.end(), X.end());
+/// @endcode
+/// First argument likely should be begin
+static auto makeUnexpectedBeginEndMatcher(unsigned Index, StringRef BindTo,
+                                          NameMatchers Names,
+                                          ArrayRef<StringRef> MakeReverse,
+                                          const NameMatchers &RevNames) {
+  return callExpr(hasArgument(Index, makeExprMatcher(expr().bind(BindTo), Names,
+                                                     MakeReverse, RevNames,
+                                                     ArgMismatchRevBind)
+                                         .bind(ArgMismatchExpr)));
+}
+
+static auto makeUnexpectedBeginEndPair(unsigned BeginExpected,
+                                       unsigned EndExpected,
+                                       NamePairs BeginEndPairs,
+                                       ArrayRef<StringRef> MakeReverse) {
+  return eachOf(makeUnexpectedBeginEndMatcher(
+                    BeginExpected, ArgMismatchBegin, BeginEndPairs.EndNames,
+                    MakeReverse, BeginEndPairs.BeginNames),
+                makeUnexpectedBeginEndMatcher(
+                    EndExpected, ArgMismatchEnd, BeginEndPairs.BeginNames,
+                    MakeReverse, BeginEndPairs.EndNames));
+}
+
+static auto makePairRangeMatcherInternal(
+    ast_matchers::internal::Matcher<NamedDecl> NameMatcher,
+    unsigned BeginExpected, unsigned EndExpected, const FullState &State) {
+  return callExpr(
+      callee(functionDecl(
+          areParametersSameTemplateType({BeginExpected, EndExpected}),
+          std::move(NameMatcher))),
+      anyOf(makeRangeArgMismatch(BeginExpected, EndExpected, State.Forward,
+                                 State.Reversed, State.MakeReverseIterator),
+            makeUnexpectedBeginEndPair(BeginExpected, EndExpected,
+                                       State.Combined,
+                                       State.MakeReverseIterator)));
+}
+
+/// The full matcher for functions that take a range with 2 arguments
+static auto makePairRangeMatcher(ArrayRef<StringRef> FuncNames,
+                                 unsigned BeginExpected, unsigned EndExpected,
+                                 const FullState &State) {
+  return makePairRangeMatcherInternal(hasAnyName(FuncNames), BeginExpected,
+                                      EndExpected, State)
+      .bind(Callee);
+}
+
+static auto makeHalfOpenMatcher(ArrayRef<StringRef> FuncNames,
+                                unsigned BeginExpected, unsigned PotentialEnd,
+                                const FullState &State) {
+  auto NameMatcher = hasAnyName(FuncNames);
+  return callExpr(
+             anyOf(makePairRangeMatcherInternal(NameMatcher, BeginExpected,
+                                                PotentialEnd, State),
+                   allOf(callee(functionDecl(NameMatcher, isParameterTypeUnique(
+                                                              BeginExpected))),
+                         makeUnexpectedBeginEndMatcher(
+                             BeginExpected, ArgMismatchBegin,
+                             State.Combined.EndNames, State.MakeReverseIterator,
+                             State.Combined.BeginNames))))
+      .bind(Callee);
+}
+
+/// Detects calls where a single output iterator is expected, yet an end of
+/// container input is supplied Usually these arguments would be supplied with
+/// things like `std::back_inserter`
+static auto makeExpectedBeginFullMatcher(ArrayRef<StringRef> FuncNames,
+                                         unsigned ExpectedIndex,
+                                         const FullState &State) {
+  return callExpr(argumentCountAtLeast(ExpectedIndex + 1),
+                  callee(functionDecl(isParameterTypeUnique(ExpectedIndex),
+                                      hasAnyName(FuncNames))),
+                  makeUnexpectedBeginEndMatcher(
+                      ExpectedIndex, OutputRangeEnd, State.Combined.EndNames,
+                      State.MakeReverseIterator, State.Combined.BeginNames))
+      .bind(Callee);
+}
+
+/// Detects calls where a single output iterator is expected, yet an end of
+/// container input is supplied Usually these arguments would be supplied with
+/// things like `std::back_inserter`
+static auto makeExpectedEndFullMatcher(ArrayRef<StringRef> FuncNames,
+                                       unsigned ExpectedIndex,
+                                       const FullState &State) {
+  return callExpr(argumentCountAtLeast(ExpectedIndex + 1),
+                  callee(functionDecl(isParameterTypeUnique(ExpectedIndex),
+                                      hasAnyName(FuncNames))),
+                  makeUnexpectedBeginEndMatcher(ExpectedIndex, OutputRangeBegin,
+                                                State.Combined.BeginNames,
+                                                State.MakeReverseIterator,
+                                                State.Combined.EndNames))
+      .bind(Callee);
+}
+
+/// Detects calls to container methods which expect an argument to be an
+/// iterator of the container
+/// @code
+///   cont_a.erase(cont_b.begin());
+/// @endcode
+static auto makeContainerInternalMatcher(ArrayRef<StringRef> ClassNames,
+                                         ArrayRef<StringRef> ClassMethods,
+                                         unsigned InternalExpected,
+                                         const FullState &State) {
+  return cxxMemberCallExpr(
+             thisPointerType(cxxRecordDecl(hasAnyName(ClassNames))),
+             callee(cxxMethodDecl(hasAnyName(ClassMethods))),
+             on(expr().bind(Internal)),
+             hasArgument(
+                 InternalExpected,
+                 makeExprMatcher(
+                     expr(unless(matchers::isStatementIdenticalToBoundNode(
+                              Internal)))
+                         .bind(InternalOther),
+                     State.All, State.MakeReverseIterator, State.All,
+                     ArgMismatchRevBind)
+                     .bind(InternalArgument)))
+      .bind(Callee);
+}
+
+/// Like @c makePairRangeMatcher but instead of operating on free functions,
+/// operates on class methods
+/// @code
+///   cont.insert(cont.begin(), other_range.begin(), other_range.end());
+/// @endcode
+static auto makeContainerPairRangeMatcher(ArrayRef<StringRef> ClassNames,
+                                          ArrayRef<StringRef> ClassMethods,
+                                          unsigned BeginExpected,
+                                          unsigned EndExpected,
+                                          const FullState &State) {
+  return cxxMemberCallExpr(
+             thisPointerType(cxxRecordDecl(hasAnyName(ClassNames))),
+             callee(cxxMethodDecl(
+                 hasAnyName(ClassMethods),
+                 areParametersSameTemplateType({BeginExpected, EndExpected}))),
+             anyOf(makeRangeArgMismatch(BeginExpected, EndExpected,
+                                        State.Forward, State.Reversed,
+                                        State.MakeReverseIterator),
+                   makeUnexpectedBeginEndPair(BeginExpected, EndExpected,
+                                              State.Combined,
+                                              State.MakeReverseIterator)))
+      .bind(Callee);
+}
+
+/// Looks for calls that advance past the end of a range or before the start of
+/// a range.
+static auto makeUnexpectedIncDecMatcher(const FullState &State, bool IsInc) {
+  auto Arg =
+      makeExprMatcher(
+          expr(), IsInc ? State.Combined.EndNames : State.Combined.BeginNames,
+          State.MakeReverseIterator,
+          IsInc ? State.Combined.BeginNames : State.Combined.EndNames,
+          ArgMismatchRevBind)
+          .bind(UnexpectedIncDecExpr);
+  return expr(
+             anyOf(
+                 callExpr(
+                     argumentCountAtLeast(1),
+                     anyOf(
+                         allOf(
+                             anyOf(unless(hasArgument(1, anything())),
+                                   hasArgument(
+                                       1, unless(isNegativeIntegerConstant()))),
+                             callee(functionDecl(hasName(
+                                 IsInc ? "::std::next" : "::std::prev")))),
+                         allOf(hasArgument(1, isNegativeIntegerConstant()),
+                               callee(functionDecl(hasName(
+                                   IsInc ? "::std::prev" : "::std::next"))))),
+                     hasArgument(0, Arg)),
+                 mapAnyOf(binaryOperator, cxxOperatorCallExpr)
+                     .with(anyOf(
+                         allOf(hasOperatorName(IsInc ? "+" : "-"), hasLHS(Arg),
+                               hasRHS(
+                                   allOf(hasType(isInteger()),
+                                         unless(isNegativeIntegerConstant())))),
+                         allOf(hasOperatorName(IsInc ? "-" : "+"), hasLHS(Arg),
+                               hasRHS(allOf(hasType(isInteger()),
+                                            isNegativeIntegerConstant()))))),
+                 cxxMemberCallExpr(
+                     anyOf(
+                         allOf(
+                             hasArgument(
+                                 0, allOf(hasType(isInteger()),
+                                          unless(isNegativeIntegerConstant()))),
+                             callee(cxxMethodDecl(
+                                 hasName(IsInc ? "operator+" : "operator-")))),
+                         allOf(
+                             hasArgument(0, allOf(hasType(isInteger()),
+                                                  isNegativeIntegerConstant())),
+                             callee(cxxMethodDecl(
+                                 hasName(IsInc ? "operator-" : "operator+"))))),
+                     on(Arg))))
+      .bind(IsInc ? UnexpectedInc : UnexpectedDec);
+}
+
+void prependStdPrefix(llvm::MutableArrayRef<std::string> Items) {
+  static constexpr llvm::StringLiteral Prefix = "::std::";
+  llvm::for_each(Items, [](std::string &Item) {
+    Item.insert(0, Prefix.data(), Prefix.size());
+  });
+}
+
+/// Gets functions that take 2 whole ranges
+static std::vector<std::string>
+getSingleRangeWithRevOutputIterator(const LangOptions &LangOpts) {
+  std::vector<std::string> Result;
+  llvm::append_range(Result, std::array{"copy_backward"});
+  if (LangOpts.CPlusPlus11) {
+    llvm::append_range(Result, std::array{"move_backward"});
+  }
+  prependStdPrefix(Result);
+  return Result;
+}
+
+/// Gets functions that take 2 whole ranges and optionally start with a policy
+static std::vector<std::string>
+getMultiRangePolicyFunctors(const LangOptions &LangOpts) {
+  std::vector<std::string> Result;
+  llvm::append_range(Result, std::array{"find_end", "find_first_of", "search",
+                                        "includes", "lexicographical_compare"});
+  if (LangOpts.CPlusPlus20)
+    llvm::append_range(Result, std::array{"lexicographical_compare_three_way"});
+  prependStdPrefix(Result);
+  return Result;
+}
+
+/// Gets a function that takes 2 ranges where the second may be specified by
+/// just a start iterator or a start/end pair, The range may optionally start
+/// with a policy
+static std::vector<std::string> getMultiRangePolicyPotentiallyHalfOpenFunctors(
+    const LangOptions & /* LangOpts */) {
+  std::vector<std::string> Result;
+  llvm::append_range(Result, std::array{"mismatch", "equal"});
+  prependStdPrefix(Result);
+  return Result;
+}
+
+static std::vector<std::string>
+getMultiRangePotentiallyHalfOpenFunctors(const LangOptions &LangOpts) {
+  std::vector<std::string> Result;
+  if (LangOpts.CPlusPlus11)
+    llvm::append_range(Result, std::array{"is_permutation"});
+  prependStdPrefix(Result);
+  return Result;
+}
+
+static std::vector<std::string> getMultiRangePolicyWithSingleOutputIterator(
+    const LangOptions & /* LangOpts */) {
+  std::vector<std::string> Result;
+  llvm::append_range(Result, std::array{"set_union", "set_intersection",
+                                        "set_difference",
+                                        "set_symmetric_difference", "merge"});
+  prependStdPrefix(Result);
+  return Result;
+}
+
+// Returns a vector of function that take a range in the first and second
+// arguments
+static std::vector<std::string>
+getSingleRangeFunctors(const LangOptions &LangOpts) {
+  std::vector<std::string> Result;
+  if (LangOpts.CPlusPlus17) {
+    llvm::append_range(Result, std::array{"sample"});
+  } else {
+    llvm::append_range(Result, std::array{"random_shuffle"});
+  }
+  if (LangOpts.CPlusPlus11) {
+    llvm::append_range(Result,
+                       std::array{"shuffle", "partition_point", "iota"});
+  }
+  llvm::append_range(Result, std::array{
+                                 "lower_bound",
+                                 "upper_bound",
+                                 "equal_range",
+                                 "binary_search",
+                                 "push_heap",
+                                 "pop_heap",
+                                 "make_heap",
+                                 "sort_heap",
+                                 "next_permutation",
+                                 "prev_permutation",
+                                 "accumulate",
+                             });
+  prependStdPrefix(Result);
+  return Result;
+}
+
+// Returns a vector of function that take a range in the first and second or
+// second and third arguments
+static std::vector<std::string>
+getSingleRangePolicyFunctors(const LangOptions &LangOpts) {
+  std::vector<std::string> Result;
+  if (LangOpts.CPlusPlus11)
+    llvm::append_range(Result, std::array{
+                                   "all_of",
+                                   "any_of",
+                                   "none_of",
+                                   "is_partitioned",
+                                   "is_sorted",
+                                   "is_sorted_until",
+                                   "is_heap",
+                                   "is_heap_until",
+                                   "minmax_element",
+                               });
+
+  if (LangOpts.CPlusPlus17)
+    llvm::append_range(Result,
+                       std::array{"reduce", "uninitialized_default_construct",
+                                  "uninitialized_value_construct", "destroy"});
+  if (LangOpts.CPlusPlus20)
+    llvm::append_range(Result, std::array{"shift_left", "shift_right"});
+
+  llvm::append_range(Result, std::array{
+                                 "find",
+                                 "find_if",
+                                 "find_if_not",
+                                 "adjacent_find",
+                                 "count",
+                                 "count_if",
+                                 "search_n",
+                                 "replace",
+                                 "replace_if",
+                                 "fill",
+                                 "generate",
+                                 "remove_if",
+                                 "unique",
+                                 "reverse",
+                                 "partition",
+                                 "stable_partition",
+                                 "sort",
+                                 "stable_sort",
+                                 "max_element",
+                                 "min_element",
+                                 "uninitialized_fill",
+                             });
+  prependStdPrefix(Result);
+  return Result;
+}
+
+static std::vector<std::string>
+getSingleRangePolicyWithSingleOutputIteratorFunctions(
+    const LangOptions &LangOpts) {
+  std::vector<std::string> Result;
+  if (LangOpts.CPlusPlus11)
+    llvm::append_range(
+        Result,
+        std::array{
+            "copy", "copy_if", "move", "swap_ranges",
+            "partition_copy", // FIXME: This will miss diagnosing the second
+                              // range output argument if a policy is specified
+        });
+  if (LangOpts.CPlusPlus17)
+    llvm::append_range(
+        Result, std::array{"exclusive_scan", "inclusive_scan",
+                           "transform_reduce", "transform_exclusive_scan",
+                           "transform_inclusive_scan", "uninitialized_move"});
+  llvm::append_range(
+      Result,
+      std::array{"transform", // FIXME: This is not ideal as transform can take
+                              // 2 different input ranges and one output
+                 "replace_copy", "replace_copy_if", "remove_copy_if",
+                 "unique_copy", "reverse_copy", "adjacent_difference",
+                 "uninitialized_copy"
+
+      });
+  prependStdPrefix(Result);
+  return Result;
+}
+
+static std::vector<std::string> getSingleRangeWithSingleOutputIteratorFunctions(
+    const LangOptions & /* LangOpts */) {
+  std::vector<std::string> Result;
+  llvm::append_range(
+      Result,
+      std::array{
+          "inner_product", // FIXME: The 3rd argument to this is an input, but
+                           // currently the warning says output
+          "partial_sum",
+      });
+  prependStdPrefix(Result);
+  return Result;
+}
+
+void IncorrectIteratorsCheck::registerMatchers(MatchFinder *Finder) {
+  NamePairs Forward{NameMatchers{BeginFree, BeginMethod},
+                    NameMatchers{EndFree, EndMethod}};
+  NamePairs Reverse{NameMatchers{RBeginFree, RBeginMethod},
+                    NameMatchers{REndFree, REndMethod}};
+  llvm::SmallVector<StringRef, 8> CombinedFreeBegin{
+      llvm::iterator_range{llvm::concat<StringRef>(BeginFree, RBeginFree)}};
+  llvm::SmallVector<StringRef, 8> CombinedFreeEnd{
+      llvm::iterator_range{llvm::concat<StringRef>(EndFree, REndFree)}};
+  llvm::SmallVector<StringRef, 8> CombinedMethodBegin{
+      llvm::iterator_range{llvm::concat<StringRef>(BeginMethod, RBeginMethod)}};
+  llvm::SmallVector<StringRef, 8> CombinedMethodEnd{
+      llvm::iterator_range{llvm::concat<StringRef>(EndMethod, REndMethod)}};
+  llvm::SmallVector<StringRef, 16> AllFree{llvm::iterator_range{
+      llvm::concat<StringRef>(CombinedFreeBegin, CombinedFreeEnd)}};
+  llvm::SmallVector<StringRef, 16> AllMethod{llvm::iterator_range{
+      llvm::concat<StringRef>(CombinedMethodBegin, CombinedMethodEnd)}};
+  NamePairs Combined{NameMatchers{CombinedFreeBegin, CombinedMethodBegin},
+                     NameMatchers{CombinedFreeEnd, CombinedMethodEnd}};
+  FullState State{
+      Forward, Reverse, Combined, {AllFree, AllMethod}, MakeReverseIterator};
+  auto SingleRange = getSingleRangeFunctors(getLangOpts());
+  auto SingleRangePolicy = getSingleRangePolicyFunctors(getLangOpts());
+  auto SingleRangePolicyHalf =
+      getSingleRangePolicyWithSingleOutputIteratorFunctions(getLangOpts());
+  auto SingleRangeHalf =
+      getSingleRangeWithSingleOutputIteratorFunctions(getLangOpts());
+  auto SingleRangeBackwardsHalf =
+      getSingleRangeWithRevOutputIterator(getLangOpts());
+  auto MultiRangePolicy = getMultiRangePolicyFunctors(getLangOpts());
+  auto MultiRangePolicyPotentiallyHalfOpen =
+      getMultiRangePolicyPotentiallyHalfOpenFunctors(getLangOpts());
+  auto MultiRangePotentiallyHalfOpen =
+      getMultiRangePotentiallyHalfOpenFunctors(getLangOpts());
+  auto MultiRangePolicySingleOutputIterator =
+      getMultiRangePolicyWithSingleOutputIterator(getLangOpts());
+
+  static const auto ToRefs =
+      [](std::initializer_list<std::reference_wrapper<std::vector<std::string>>>
+             Items) {
+        std::vector<StringRef> Result;
+        for (const auto &Item : Items) {
+          llvm::append_range(Result, Item.get());
+        }
+        return Result;
+      };
+
+  Finder->addMatcher(
+      makePairRangeMatcher(
+          ToRefs({SingleRange, SingleRangeHalf, SingleRangePolicy,
+                  SingleRangePolicyHalf, SingleRangeBackwardsHalf,
+                  MultiRangePolicy, MultiRangePotentiallyHalfOpen,
+                  MultiRangePolicyPotentiallyHalfOpen,
+                  MultiRangePolicySingleOutputIterator}),
+          0, 1, State),
+      this);
+  Finder->addMatcher(
+      makePairRangeMatcher(
+          ToRefs({SingleRangePolicy, SingleRangePolicyHalf, MultiRangePolicy,
+                  MultiRangePolicyPotentiallyHalfOpen,
+                  MultiRangePolicySingleOutputIterator}),
+          1, 2, State),
+      this);
+  Finder->addMatcher(
+      makeExpectedBeginFullMatcher(
+          ToRefs({SingleRangeHalf, SingleRangePolicyHalf}), 2, State),
+      this);
+  Finder->addMatcher(
+      makeExpectedBeginFullMatcher(ToRefs({SingleRangePolicyHalf}), 3, State),
+      this);
+  Finder->addMatcher(
+      makeExpectedEndFullMatcher(ToRefs({SingleRangeBackwardsHalf}), 2, State),
+      this);
+  Finder->addMatcher(
+      makePairRangeMatcher(ToRefs({MultiRangePolicy}), 2, 3, State), this);
+  Finder->addMatcher(
+      makePairRangeMatcher(ToRefs({MultiRangePolicy}), 3, 4, State), this);
+  Finder->addMatcher(
+      makeHalfOpenMatcher(ToRefs({MultiRangePotentiallyHalfOpen,
+                                  MultiRangePolicyPotentiallyHalfOpen,
+                                  MultiRangePolicySingleOutputIterator}),
+                          2, 3, State),
+      this);
+  Finder->addMatcher(
+      makeHalfOpenMatcher(ToRefs({MultiRangePolicyPotentiallyHalfOpen,
+                                  MultiRangePolicySingleOutputIterator}),
+                          3, 4, State),
+      this);
+  Finder->addMatcher(
+      makeExpectedBeginFullMatcher(
+          ToRefs({MultiRangePolicySingleOutputIterator}), 4, State),
+      this);
+  Finder->addMatcher(
+      makeExpectedBeginFullMatcher(
+          ToRefs({MultiRangePolicySingleOutputIterator}), 5, State),
+      this);
+
+  Finder->addMatcher(
+      makeContainerPairRangeMatcher({"::std::vector", "::std::list"},
+                                    {"insert"}, 1, 2, State),
+      this);
+  Finder->addMatcher(
+      makeContainerInternalMatcher({"::std::vector", "::std::list"},
+                                   {"insert", "erase", "emplace"}, 0, State),
+      this);
+  for (bool IsInc : {true, false})
+    Finder->addMatcher(makeUnexpectedIncDecMatcher(State, IsInc), this);
+}
+
+void IncorrectIteratorsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>(Callee);
+
+  auto AddRevNote = [&Result, this](bool IsBegin) {
+    if (const auto *Rev =
+            Result.Nodes.getNodeAs<CallExpr>(ArgMismatchRevBind)) {
+      diag(Rev->getBeginLoc(),
+           IsBegin ? "%0 changes 'begin' into a 'end' iterator"
+                   : "%0 changes 'end' into a 'begin' iterator",
+           DiagnosticIDs::Note)
+          << Rev->getSourceRange() << Rev->getDirectCallee();
+    }
+  };
+
+  if (const auto *BeginMismatch =
+          Result.Nodes.getNodeAs<Expr>(ArgMismatchBegin)) {
+    diag(BeginMismatch->getBeginLoc(),
+         "'end' iterator supplied where a 'begin' iterator is expected")
+        << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange();
+    AddRevNote(/*IsBegin=*/true);
+    return;
+  }
+
+  if (const auto *EndMismatch = Result.Nodes.getNodeAs<Expr>(ArgMismatchEnd)) {
+    diag(EndMismatch->getBeginLoc(),
+         "'begin' iterator supplied where an 'end' iterator is expected")
+        << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange();
+    AddRevNote(/*IsBegin=*/false);
+    return;
+  }
+
+  if (const auto *InternalArg =
+          Result.Nodes.getNodeAs<Expr>(InternalArgument)) {
+    diag(InternalArg->getBeginLoc(),
+         "%0 called with an iterator for a different container")
+        << Call->getDirectCallee();
+    const auto *Object = Result.Nodes.getNodeAs<Expr>(Internal);
+    diag(Object->getBeginLoc(), "container is specified here",
+         DiagnosticIDs::Note)
+        << Object->getSourceRange();
+    const auto *Other = Result.Nodes.getNodeAs<Expr>(InternalOther);
+    diag(Other->getBeginLoc(), "different container provided here",
+         DiagnosticIDs::Note)
+        << Other->getSourceRange();
+    return;
+  }
+
+  if (const auto *Range1 = Result.Nodes.getNodeAs<Expr>(FirstRangeArg)) {
+    const auto *Range2 = Result.Nodes.getNodeAs<Expr>(SecondRangeArg);
+    const auto *FullRange1 = Result.Nodes.getNodeAs<Expr>(FirstRangeArgExpr);
+    const auto *FullRange2 = Result.Nodes.getNodeAs<Expr>(SecondRangeArgExpr);
+    assert(Range1 && Range2 && FullRange1 && FullRange2 && "Unexpected match");
+    diag(Call->getBeginLoc(), "mismatched ranges pass to function")
+        << FullRange1->getSourceRange() << FullRange2->getSourceRange();
+    diag(Range1->getBeginLoc(), "first range passed here to begin",
+         DiagnosticIDs::Note)
+        << FullRange1->getSourceRange();
+    diag(Range2->getBeginLoc(), "different range passed here to end",
+         DiagnosticIDs::Note)
+        << FullRange2->getSourceRange();
+    return;
+  }
+
+  if (const auto *OutputMismatch =
+          Result.Nodes.getNodeAs<Expr>(OutputRangeEnd)) {
+    diag(OutputMismatch->getBeginLoc(),
+         "'end' iterator supplied where an output iterator is expected")
+        << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange();
+    AddRevNote(/*IsBegin=*/true);
+    return;
+  }
+
+  if (const auto *OutputMismatch =
+          Result.Nodes.getNodeAs<Expr>(OutputRangeBegin)) {
+    diag(OutputMismatch->getBeginLoc(),
+         "'begin' iterator supplied where an 'end' iterator is expected")
+        << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange();
+    AddRevNote(/*IsBegin=*/true);
+    return;
+  }
+
+  if (const auto *BadInc = Result.Nodes.getNodeAs<Expr>(UnexpectedInc)) {
+    diag(BadInc->getExprLoc(), "trying to increment past the end of a range")
+        << Result.Nodes.getNodeAs<Expr>(UnexpectedIncDecExpr)->getSourceRange();
+    AddRevNote(/*IsBegin=*/true);
+    return;
+  }
+
+  if (const auto *BadDec = Result.Nodes.getNodeAs<Expr>(UnexpectedDec)) {
+    diag(BadDec->getExprLoc(),
+         "trying to decrement before the start of a range")
+        << Result.Nodes.getNodeAs<Expr>(UnexpectedIncDecExpr)->getSourceRange();
+    AddRevNote(/*IsBegin=*/false);
+    return;
+  }
+  llvm_unreachable("Unhandled matches");
+}
+
+IncorrectIteratorsCheck::IncorrectIteratorsCheck(StringRef Name,
+                                                 ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      BeginFree(utils::options::parseStringList(
+          Options.get("BeginFree", "::std::begin;::std::cbegin"))),
+      EndFree(utils::options::parseStringList(
+          Options.get("EndFree", "::std::end;::std::cend"))),
+      BeginMethod(utils::options::parseStringList(
+          Options.get("BeginMethod", "begin;cbegin"))),
+      EndMethod(utils::options::parseStringList(
+          Options.get("EndMethod", "end;cend"))),
+      RBeginFree(utils::options::parseStringList(
+          Options.get("RBeginFree", "::std::rbegin;::std::crbegin"))),
+      REndFree(utils::options::parseStringList(
+          Options.get("REndFree", "::std::rend;::std::crend"))),
+      RBeginMethod(utils::options::parseStringList(
+          Options.get("RBeginMethod", "rbegin;crbegin"))),
+      REndMethod(utils::options::parseStringList(
+          Options.get("REndMethod", "rend;crend"))),
+      MakeReverseIterator(utils::options::parseStringList(
+          Options.get("MakeReverseIterator", "::std::make_reverse_iterator"))) {
+}
+
+void IncorrectIteratorsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "BeginFree",
+                utils::options::serializeStringList(BeginFree));
+  Options.store(Opts, "EndFree", utils::options::serializeStringList(EndFree));
+  Options.store(Opts, "BeginMethod",
+                utils::options::serializeStringList(BeginMethod));
+  Options.store(Opts, "EndMethod",
+                utils::options::serializeStringList(EndMethod));
+  Options.store(Opts, "RBeginFree",
+                utils::options::serializeStringList(RBeginFree));
+  Options.store(Opts, "REndFree",
+                utils::options::serializeStringList(REndFree));
+  Options.store(Opts, "RBeginMethod",
+                utils::options::serializeStringList(RBeginMethod));
+  Options.store(Opts, "REndMethod",
+                utils::options::serializeStringList(REndMethod));
+  Options.store(Opts, "MakeReverseIterator",
+                utils::options::serializeStringList(MakeReverseIterator));
+}
+
+std::optional<TraversalKind>
+IncorrectIteratorsCheck::getCheckTraversalKind() const {
+  return TK_IgnoreUnlessSpelledInSource;
+}
+
+bool IncorrectIteratorsCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h
new file mode 100644
index 0000000000000..a57e486dc60b9
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h
@@ -0,0 +1,45 @@
+//===--- IncorrectIteratorsCheck.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_INCORRECTITERATORSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTITERATORSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects calls to iterator algorithms that are called with potentially
+/// invalid arguments.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/incorrect-iterators.html
+class IncorrectIteratorsCheck : public ClangTidyCheck {
+public:
+  IncorrectIteratorsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+private:
+  std::vector<StringRef> BeginFree;
+  std::vector<StringRef> EndFree;
+  std::vector<StringRef> BeginMethod;
+  std::vector<StringRef> EndMethod;
+  std::vector<StringRef> RBeginFree;
+  std::vector<StringRef> REndFree;
+  std::vector<StringRef> RBeginMethod;
+  std::vector<StringRef> REndMethod;
+  std::vector<StringRef> MakeReverseIterator;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTITERATORSCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 642ad39cc0c1c..6b9f6c12696f3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-incorrect-iterators
+  <clang-tidy/checks/bugprone/incorrect-iterators>` check.
+
+  Detects calls to iterator algorithms that are called with potentially
+  invalid arguments.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst
new file mode 100644
index 0000000000000..b9e4e8a7545dc
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst
@@ -0,0 +1,131 @@
+.. title:: clang-tidy - bugprone-incorrect-iterators
+
+bugprone-incorrect-iterators
+============================
+
+Detects calls to iterator algorithms where they are called with potentially
+invalid arguments.
+
+Different ranges
+================
+
+Looks for calls where the range for the ``begin`` argument is different to the
+``end`` argument.
+
+.. code-block:: c++
+
+  std::find(a.begin(), b.end(), 0);
+  std::find(std::begin(a), std::end(b));
+
+Mismatched Begin/End
+====================
+
+Looks for calls where the ``begin`` parameter is passed as an ``end`` argument or
+vice versa.
+
+.. code-block:: c++
+
+  std::find(a.begin(), a.begin(), 0); // Second argument should be a.end().
+  std::find(a.end(), a.end(), 0); // First argument should be a.begin().
+
+Container Methods
+=================
+
+Looks for calls to methods on containers that expect an iterator inside the
+container but are given a different container.
+
+.. code-block:: c++
+
+  vec.insert(other.begin(), 5); // The iterator is invalid for this container.
+
+Output Iterators
+================
+
+Looks for calls which accept a single output iterator but are passed the end of
+a container.
+
+.. code-block:: c++
+
+  std::copy(correct.begin(), correct.end(), incorrect.end());
+
+Iterator Advancing
+==================
+
+Looks for calls that advance an iterator outside its range.
+
+.. code-block:: c++
+
+  auto Iter = std::next(Cont.end());
+
+Reverse Iteration
+=================
+
+The check understands ``rbegin`` and ``rend`` and ensures they are in the
+correct places.
+
+.. code-block:: c++
+
+  std::find(a.rbegin(), a.rend(), 0); // OK.
+  std::find(a.rend(), a.rbegin(), 0); // Arguments are swapped.
+
+Manually creating a reverse iterator using the ``std::make_reverse_iterator`` is
+also supported, In this case the check looks for calls to ``end`` for the
+``begin`` parameter and vice versa. The name of functions for creating reverse
+iterator can be configured with the option :option:`MakeReverseIterator`.
+
+.. code-block:: c++
+
+  std::find(std::make_reverse_iterator(a.begin()),
+            std::make_reverse_iterator(a.end()), 0); // Arguments are swapped.
+  std::find(std::make_reverse_iterator(a.end()),
+            std::make_reverse_iterator(a.begin()), 0); // OK.
+  // Understands this spaghetti looking code is actually doing the correct thing.
+  std::find(a.rbegin(), std::make_reverse_iterator(a.begin()), 0);
+
+Options
+-------
+
+.. option:: BeginFree
+
+  A semi-colon seperated list of free function names that return an iterator to
+  the start of a range. Default value is `::std::begin;std::cbegin`.
+
+.. option:: EndFree
+
+  A semi-colon seperated list of free function names that return an iterator to
+  the end of a range. Default value is `::std::end;std::cend`.
+
+.. option:: BeginMethod
+
+  A semi-colon seperated list of method names that return an iterator to
+  the start of a range. Default value is `begin;cbegin`.
+
+.. option:: EndMethod
+
+  A semi-colon seperated list of method names that return an iterator to
+  the end of a range. Default value is `end;cend`.
+
+.. option:: RBeginFree
+
+  A semi-colon seperated list of free function names that return a reverse 
+  iterator to the start of a range. Default value is `::std::rbegin;std::crbegin`.
+
+.. option:: REndFree
+
+  A semi-colon seperated list of free function names that return a reverse 
+  iterator to the end of a range. Default value is `::std::rend;std::crend`.
+
+.. option:: RBeginMethod
+
+  A semi-colon seperated list of method names that return a reverse 
+  iterator to the start of a range. Default value is `rbegin;crbegin`.
+
+.. option:: REndMethod
+
+  A semi-colon seperated list of method names that return a reverse 
+  iterator to the end of a range. Default value is `rend;crend`.
+
+.. option:: MakeReverseIterator
+
+  A semi-colon seperated list of free functions that convert an interator into a
+  reverse iterator. Default value is `::std::make_reverse_iterator`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index a931ebf025a10..749a5454575d5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -100,6 +100,7 @@ Clang-Tidy Checks
    :doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
    :doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,
    :doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes"
+   :doc:`bugprone-incorrect-iterators <bugprone/incorrect-iterators>`,
    :doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`,
    :doc:`bugprone-infinite-loop <bugprone/infinite-loop>`,
    :doc:`bugprone-integer-division <bugprone/integer-division>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-iterators.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-iterators.cpp
new file mode 100644
index 0000000000000..12ecdf5290db2
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-iterators.cpp
@@ -0,0 +1,282 @@
+// RUN: %check_clang_tidy -std=c++14 %s bugprone-incorrect-iterators %t
+
+namespace std {
+namespace execution {
+class parallel_policy {};
+constexpr parallel_policy par;
+} // namespace execution
+
+template <typename BiDirIter> class reverse_iterator {
+public:
+  constexpr explicit reverse_iterator(BiDirIter Iter);
+  reverse_iterator operator+(int) const;
+  reverse_iterator operator-(int) const;
+};
+
+template <typename InputIt> InputIt next(InputIt Iter, int n = 1);
+template <typename BidirIt> BidirIt prev(BidirIt Iter, int n = 1);
+
+template <typename BiDirIter>
+reverse_iterator<BiDirIter> make_reverse_iterator(BiDirIter Iter);
+
+template <typename T> class vector {
+public:
+  using iterator = T *;
+  using const_iterator = const T *;
+  using reverse_iterator = std::reverse_iterator<iterator>;
+  using reverse_const_iterator = std::reverse_iterator<const_iterator>;
+
+  constexpr const_iterator begin() const;
+  constexpr const_iterator end() const;
+  constexpr const_iterator cbegin() const;
+  constexpr const_iterator cend() const;
+  constexpr iterator begin();
+  constexpr iterator end();
+  constexpr reverse_const_iterator rbegin() const;
+  constexpr reverse_const_iterator rend() const;
+  constexpr reverse_const_iterator crbegin() const;
+  constexpr reverse_const_iterator crend() const;
+  constexpr reverse_iterator rbegin();
+  constexpr reverse_iterator rend();
+
+  template <class InputIt>
+  iterator insert(const_iterator pos, InputIt first, InputIt last);
+};
+
+template <typename Container> constexpr auto begin(const Container &Cont) {
+  return Cont.begin();
+}
+
+template <typename Container> constexpr auto begin(Container &Cont) {
+  return Cont.begin();
+}
+
+template <typename Container> constexpr auto end(const Container &Cont) {
+  return Cont.end();
+}
+
+template <typename Container> constexpr auto end(Container &Cont) {
+  return Cont.end();
+}
+
+template <typename Container> constexpr auto cbegin(const Container &Cont) {
+  return Cont.cbegin();
+}
+
+template <typename Container> constexpr auto cend(const Container &Cont) {
+  return Cont.cend();
+}
+
+template <typename Container> constexpr auto rbegin(const Container &Cont) {
+  return Cont.rbegin();
+}
+
+template <typename Container> constexpr auto rbegin(Container &Cont) {
+  return Cont.rbegin();
+}
+
+template <typename Container> constexpr auto rend(const Container &Cont) {
+  return Cont.rend();
+}
+
+template <typename Container> constexpr auto rend(Container &Cont) {
+  return Cont.rend();
+}
+
+template <typename Container> constexpr auto crbegin(const Container &Cont) {
+  return Cont.crbegin();
+}
+
+template <typename Container> constexpr auto crend(const Container &Cont) {
+  return Cont.crend();
+}
+// Find
+template <class InputIt, class T>
+InputIt find(InputIt first, InputIt last, const T &value);
+
+template <class Policy, class InputIt, class T>
+InputIt find(Policy &&policy, InputIt first, InputIt last, const T &value);
+
+template <class InputIt1, class InputIt2, class OutputIt>
+OutputIt set_union(InputIt1 first1, InputIt1 last1, InputIt2 first2,
+                   InputIt2 last2, OutputIt d_first);
+
+template <class ExecutionPolicy, class ForwardIt1, class ForwardIt2,
+          class ForwardIt3>
+ForwardIt3 set_union(ExecutionPolicy &&policy, ForwardIt1 first1,
+                     ForwardIt1 last1, ForwardIt2 first2, ForwardIt2 last2,
+                     ForwardIt3 d_first);
+
+template <class RandomIt> void push_heap(RandomIt first, RandomIt last);
+
+template <class BidirIt1, class BidirIt2>
+BidirIt2 copy_backward(BidirIt1 first, BidirIt1 last, BidirIt2 d_last);
+
+template <class ForwardIt1, class ForwardIt2>
+ForwardIt1 find_end(ForwardIt1 first, ForwardIt1 last, ForwardIt2 s_first,
+                    ForwardIt2 s_last);
+
+template <class ExecutionPolicy, class ForwardIt1, class ForwardIt2>
+ForwardIt1 find_end(ExecutionPolicy &&policy, ForwardIt1 first, ForwardIt1 last,
+                    ForwardIt2 s_first, ForwardIt2 s_last);
+
+template <class InputIt1, class InputIt2>
+bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2);
+
+template <class ExecutionPolicy, class ForwardIt1, class ForwardIt2>
+bool equal(ExecutionPolicy &&policy, ForwardIt1 first1, ForwardIt1 last1,
+           ForwardIt2 first2);
+
+template <class InputIt1, class InputIt2>
+bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2);
+
+template <class ExecutionPolicy, class ForwardIt1, class ForwardIt2>
+bool equal(ExecutionPolicy &&policy, ForwardIt1 first1, ForwardIt1 last1,
+           ForwardIt2 first2, ForwardIt2 last2);
+
+template <class ForwardIt1, class ForwardIt2>
+bool is_permutation(ForwardIt1 first1, ForwardIt1 last1, ForwardIt2 first2);
+
+template <class ForwardIt1, class ForwardIt2>
+bool is_permutation(ForwardIt1 first1, ForwardIt1 last1, ForwardIt2 first2,
+                    ForwardIt2 last2);
+
+template <class InputIt, class NoThrowForwardIt>
+NoThrowForwardIt uninitialized_copy(InputIt first, InputIt last,
+                                    NoThrowForwardIt d_first);
+
+template <class ExecutionPolicy, class ForwardIt, class NoThrowForwardIt>
+NoThrowForwardIt uninitialized_copy(ExecutionPolicy &&policy, ForwardIt first,
+                                    ForwardIt last, NoThrowForwardIt d_first);
+
+template <class InputIt, class OutputIt>
+OutputIt partial_sum(InputIt first, InputIt last, OutputIt d_first);
+
+} // namespace std
+
+void Test() {
+  std::vector<int> I;
+  std::vector<int> J;
+  std::vector<int> K;
+  std::find(I.end(), I.begin(), 0);
+  // CHECK-NOTES: :[[@LINE-1]]:13: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:22: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  std::find(std::execution::par, I.begin(), J.end(), 0);
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:34: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:45: note: different range passed here to end
+  std::find(std::make_reverse_iterator(I.end()),
+            std::make_reverse_iterator(I.end()), 0);
+  // CHECK-NOTES: :[[@LINE-1]]:40: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:13: note: 'make_reverse_iterator<int *>' changes 'end' into a 'begin' iterator
+  std::find(I.rbegin(), std::make_reverse_iterator(J.begin()), 0);
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:13: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:52: note: different range passed here to end
+
+  I.insert(J.begin(), J.end(), J.begin());
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'insert<int *>' called with an iterator for a different container
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: container is specified here
+  // CHECK-NOTES: :[[@LINE-3]]:12: note: different container provided here
+  // CHECK-NOTES: :[[@LINE-4]]:23: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  // CHECK-NOTES: :[[@LINE-5]]:32: warning: 'begin' iterator supplied where an 'end' iterator is expected
+
+  std::set_union(I.begin(), I.begin(), J.end(), J.end(), K.end());
+  // CHECK-NOTES: :[[@LINE-1]]:29: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:40: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  // CHECK-NOTES: :[[@LINE-3]]:58: warning: 'end' iterator supplied where an output iterator is expected
+  std::set_union(std::execution::par, I.begin(), I.begin(), J.end(), J.end(),
+                 K.end());
+  // CHECK-NOTES: :[[@LINE-2]]:50: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-3]]:61: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  // CHECK-NOTES: :[[@LINE-3]]:18: warning: 'end' iterator supplied where an output iterator is expected
+
+  std::push_heap(std::begin(I), std::end(J));
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:29: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:42: note: different range passed here to end
+
+  std::copy_backward(I.begin(), J.end(), K.begin());
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:22: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:33: note: different range passed here to end
+  // CHECK-NOTES: :[[@LINE-4]]:42: warning: 'begin' iterator supplied where an 'end' iterator is expected
+
+  std::find_end(I.rbegin(), I.rbegin(), J.end(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:29: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:41: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  std::find_end(std::execution::par, I.rbegin(), I.rbegin(), J.end(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:50: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:62: warning: 'end' iterator supplied where a 'begin' iterator is expected
+
+  std::equal(I.begin(), I.end(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:34: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  std::equal(std::execution::par, I.begin(), I.end(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:55: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  std::equal(I.begin(), I.end(), J.rbegin(), K.rend());
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:34: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:46: note: different range passed here to end
+  std::equal(std::execution::par, I.begin(), I.end(), J.begin(), K.end());
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:55: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:66: note: different range passed here to end
+
+  std::is_permutation(I.begin(), J.end(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:23: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:34: note: different range passed here to end
+  // CHECK-NOTES: :[[@LINE-4]]:43: warning: 'end' iterator supplied where a 'begin' iterator is expected
+  std::is_permutation(I.begin(), I.end(), J.begin(), K.end());
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: mismatched ranges pass to function
+  // CHECK-NOTES: :[[@LINE-2]]:43: note: first range passed here to begin
+  // CHECK-NOTES: :[[@LINE-3]]:54: note: different range passed here to end
+
+  std::uninitialized_copy(I.begin(), I.end(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:47: warning: 'end' iterator supplied where an output iterator is expected
+  std::uninitialized_copy(std::execution::par, I.begin(), I.begin(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:59: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:70: warning: 'end' iterator supplied where an output iterator is expected
+
+  std::partial_sum(I.begin(), I.begin(), J.end());
+  // CHECK-NOTES: :[[@LINE-1]]:31: warning: 'begin' iterator supplied where an 'end' iterator is expected
+  // CHECK-NOTES: :[[@LINE-2]]:42: warning: 'end' iterator supplied where an output iterator is expected
+
+  std::next(I.end());
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: trying to increment past the end of a range
+  std::prev(I.end()); // OK
+  I.end() + 1;
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: trying to increment past the end of a range
+  I.end() - 1; // OK
+  I.rbegin() - 1;
+  // CHECK-NOTES: :[[@LINE-1]]:14: warning: trying to decrement before the start of a range
+  I.rbegin() + 1; // OK
+  I.rbegin().operator-(1);
+  // CHECK-NOTES: :[[@LINE-1]]:14: warning: trying to decrement before the start of a range
+
+  std::next(I.end(),
+            -1); // Technically OK, getting the previous is a weird way.
+  std::next(I.end(), 1);
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: trying to increment past the end of a range
+  std::next(I.begin(), 1); // OK
+  std::next(I.begin(), -1);
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: trying to decrement before the start of a range
+  std::prev(I.end(), -1);
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: trying to increment past the end of a range
+  std::prev(I.end(), 1);    // OK
+  std::prev(I.begin(), -1); // Technially OK, getting next in a weird way.
+  std::prev(I.begin(), 1);
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: trying to decrement before the start of a range
+
+  I.end() - -1;
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: trying to increment past the end of a range
+  I.begin() + -1;
+  // CHECK-NOTES: :[[@LINE-1]]:13: warning: trying to decrement before the start of a range
+
+  std::make_reverse_iterator(I.begin()) - -1;
+  // CHECK-NOTES: :[[@LINE-1]]:41: warning: trying to increment past the end of a range
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: 'make_reverse_iterator<int *>' changes 'begin' into a 'end' iterator
+  std::make_reverse_iterator(I.end()) + -1;
+  // CHECK-NOTES: :[[@LINE-1]]:39: warning: trying to decrement before the start of a range
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: 'make_reverse_iterator<int *>' changes 'end' into a 'begin' iterator
+}



More information about the cfe-commits mailing list