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

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 26 11:35:09 PDT 2024


================
@@ -0,0 +1,756 @@
+//===--- 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/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/ASTMatchersMacros.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.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/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;
+}
+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 auto
+makeExprMatcher(ast_matchers::internal::Matcher<Expr> ArgumentMatcher,
+                const NameMatchers &Names, ArrayRef<StringRef> MakeReverse,
+                const NameMatchers &RevNames, StringRef RevBind) {
+  return expr(anyOf(
+      cxxMemberCallExpr(argumentCountIs(0),
+                        callee(cxxMethodDecl(hasAnyName(Names.MethodNames))),
+                        on(ArgumentMatcher)),
+      callExpr(argumentCountIs(1),
+               hasDeclaration(functionDecl(hasAnyName(Names.FreeNames))),
+               hasArgument(0, ArgumentMatcher)),
+      callExpr(
+          callee(functionDecl(hasAnyName(MakeReverse))), argumentCountIs(1),
+          hasArgument(
+              0, expr(anyOf(cxxMemberCallExpr(argumentCountIs(0),
+                                              callee(cxxMethodDecl(hasAnyName(
+                                                  RevNames.MethodNames))),
+                                              on(ArgumentMatcher)),
+                            callExpr(argumentCountIs(1),
+                                     hasDeclaration(functionDecl(
+                                         hasAnyName(RevNames.FreeNames))),
+                                     hasArgument(0, ArgumentMatcher))))))
+          .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));
+}
+
+/// The full matcher for functions that take a range with 2 arguments
+static auto makePairRangeMatcher(std::vector<StringRef> FuncNames,
+                                 unsigned BeginExpected, unsigned EndExpected,
+                                 const FullState &State) {
+
+  return callExpr(callee(functionDecl(hasAnyName(std::move(FuncNames)),
+                                      areParametersSameTemplateType(
+                                          {BeginExpected, EndExpected}))),
+                  anyOf(makeRangeArgMismatch(BeginExpected, EndExpected,
+                                             State.Forward, State.Reversed,
+                                             State.MakeReverseIterator),
+                        makeUnexpectedBeginEndPair(BeginExpected, EndExpected,
+                                                   State.Combined,
+                                                   State.MakeReverseIterator)))
+      .bind(Callee);
+}
+
+static auto makeHalfOpenMatcher(std::vector<StringRef> FuncNames,
+                                unsigned BeginExpected, unsigned PotentialEnd,
+                                const FullState &State) {
+  auto NameMatcher = hasAnyName(std::move(FuncNames));
+  return callExpr(
+             anyOf(allOf(callee(functionDecl(
+                             NameMatcher, areParametersSameTemplateType(
+                                              {BeginExpected, PotentialEnd}))),
+                         anyOf(makeRangeArgMismatch(
+                                   BeginExpected, PotentialEnd, State.Forward,
+                                   State.Reversed, State.MakeReverseIterator),
+                               makeUnexpectedBeginEndPair(
+                                   BeginExpected, PotentialEnd, State.Combined,
+                                   State.MakeReverseIterator))),
----------------
5chmidti wrote:

This `allOf` does the same as `makePairRangeMatcher` above.

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


More information about the cfe-commits mailing list