[clang-tools-extra] [clang-tidy] Add readability-use-span-first-last check (PR #118074)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 3 00:13:22 PST 2024


================
@@ -0,0 +1,125 @@
+//===--- UseSpanFirstLastCheck.cpp - 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseSpanFirstLastCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) {
+  // Match span::subspan calls
+  const auto HasSpanType =
+      hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+          classTemplateSpecializationDecl(hasName("::std::span"))))));
+
+  Finder->addMatcher(cxxMemberCallExpr(callee(memberExpr(hasDeclaration(
+                                           cxxMethodDecl(hasName("subspan"))))),
+                                       on(expr(HasSpanType)))
+                         .bind("subspan"),
+                     this);
+}
+
+void UseSpanFirstLastCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("subspan");
+  if (!Call)
+    return;
+
+  handleSubspanCall(Result, Call);
+}
+
+void UseSpanFirstLastCheck::handleSubspanCall(
+    const MatchFinder::MatchResult &Result, const CXXMemberCallExpr *Call) {
+  unsigned NumArgs = Call->getNumArgs();
+  if (NumArgs == 0 || NumArgs > 2)
+    return;
+
+  const Expr *Offset = Call->getArg(0);
+  const Expr *Count = NumArgs > 1 ? Call->getArg(1) : nullptr;
+  auto &Context = *Result.Context;
+
+  class SubspanVisitor : public RecursiveASTVisitor<SubspanVisitor> {
----------------
5chmidti wrote:

(on mobile, so I can't check)

The visitor is overkill and will, most importantly, produce false positives: `s.subspan(0, 100 + (s.size() - n))`, because the visitor only searches for the specific pattern being contained in the arg, and not that it is actually the arg itself. It would be much cleaner to check this in the matcher: `anyOf(callExpr(has argument(0, integer literal(equals(0)).bind("zero_offset"), /*n arg*/), callExpr(/*size-n arg*/)`

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


More information about the cfe-commits mailing list