https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/182027
>From 0cbb04f94ad91bbf18004c95f7196a23ea558734 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <[email protected]> Date: Wed, 18 Feb 2026 15:01:54 +0100 Subject: [PATCH] [clang-tidy] Add modernize-use-span-param check Finds function parameters declared as const std::vector<T>& that are only used for read-only element access, and suggests using std::span<const T> instead. This makes the interface more generic, allowing callers to pass arrays, spans, or other contiguous ranges without requiring a vector. --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 2 + .../modernize/UseSpanParamCheck.cpp | 213 ++++++++++++++++++ .../clang-tidy/modernize/UseSpanParamCheck.h | 40 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 7 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize/use-span-param.rst | 45 ++++ .../checkers/modernize/use-span-param.cpp | 78 +++++++ 8 files changed, 387 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-param.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-span-param.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index cc4cc7a02b594..8cbda5d5afc7f 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -46,6 +46,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseOverrideCheck.cpp UseRangesCheck.cpp UseScopedLockCheck.cpp + UseSpanParamCheck.cpp UseStartsEndsWithCheck.cpp UseStdFormatCheck.cpp UseStdNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index fcb860d8c5298..e5ac3f3625e02 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -46,6 +46,7 @@ #include "UseOverrideCheck.h" #include "UseRangesCheck.h" #include "UseScopedLockCheck.h" +#include "UseSpanParamCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" @@ -94,6 +95,7 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseRangesCheck>("modernize-use-ranges"); CheckFactories.registerCheck<UseScopedLockCheck>( "modernize-use-scoped-lock"); + CheckFactories.registerCheck<UseSpanParamCheck>("modernize-use-span-param"); CheckFactories.registerCheck<UseStartsEndsWithCheck>( "modernize-use-starts-ends-with"); CheckFactories.registerCheck<UseStdFormatCheck>("modernize-use-std-format"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.cpp new file mode 100644 index 0000000000000..0098ccbf12050 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.cpp @@ -0,0 +1,213 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UseSpanParamCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/StringSet.h" +#include <cassert> + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +UseSpanParamCheck::UseSpanParamCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Inserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} + +void UseSpanParamCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", Inserter.getStyle()); +} + +void UseSpanParamCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +// Methods on std::vector that only read data (compatible with std::span). +static bool isReadOnlyVectorMethod(StringRef Name) { + static const llvm::StringSet<> ReadOnlyMethods = { + "operator[]", "at", "data", "size", "empty", + "begin", "end", "cbegin", "cend", "rbegin", + "rend", "crbegin", "crend", "front", "back"}; + return ReadOnlyMethods.contains(Name); +} + +// Check if all uses of the parameter in the function body are read-only. +static bool allUsesAreReadOnly(const ParmVarDecl *Param, + const FunctionDecl *Func, ASTContext &Context) { + const Stmt *Body = Func->getBody(); + if (!Body) + return false; + + const auto Refs = match( + findAll(declRefExpr(to(equalsNode(Param))).bind("ref")), *Body, Context); + + for (const auto &Ref : Refs) { + const auto *DRE = Ref.getNodeAs<DeclRefExpr>("ref"); + if (!DRE) + return false; + + // Walk up through implicit casts and parens to find the "real" parent. + const Expr *Current = DRE; + while (true) { + const auto Parents = Context.getParents(*Current); + if (Parents.empty()) + return false; + const auto &Parent = Parents[0]; + if (const auto *ICE = Parent.get<ImplicitCastExpr>()) { + Current = ICE; + continue; + } + if (const auto *PE = Parent.get<ParenExpr>()) { + Current = PE; + continue; + } + + // Member call on the vector: check it's a read-only method. + if (const auto *MCE = Parent.get<CXXMemberCallExpr>()) { + const CXXMethodDecl *Method = MCE->getMethodDecl(); + if (!Method || !isReadOnlyVectorMethod(Method->getName())) + return false; + break; + } + + // Operator[] via CXXOperatorCallExpr. + if (const auto *OCE = Parent.get<CXXOperatorCallExpr>()) { + if (OCE->getOperator() == OO_Subscript) + break; + return false; + } + + // Used in a range-based for loop: the DRE is inside the implicit + // __range variable's initializer, so the parent is a VarDecl. + if (const auto *VD = Parent.get<VarDecl>()) { + if (VD->isImplicit()) { + // Check that the implicit VarDecl is the range variable of a + // CXXForRangeStmt. + const auto VDParents = Context.getParents(*VD); + for (const auto &VDP : VDParents) { + if (const auto *DS = VDP.get<DeclStmt>()) { + const auto DSParents = Context.getParents(*DS); + for (const auto &DSP : DSParents) + if (DSP.get<CXXForRangeStmt>()) + goto range_ok; + } + } + } + return false; + range_ok: + break; + } + + // Member expression (e.g. v.size()) - walk further up. + if (Parent.get<MemberExpr>()) { + Current = Parent.get<MemberExpr>(); + continue; + } + + // Passed as argument to a function - check parameter type. + if (const auto *CE = Parent.get<CallExpr>()) { + const FunctionDecl *Callee = CE->getDirectCallee(); + if (!Callee) + return false; + // Find which argument position this is. + bool Found = false; + for (unsigned I = 0; I < CE->getNumArgs(); ++I) { + if (CE->getArg(I)->IgnoreParenImpCasts() == DRE) { + if (I < Callee->getNumParams()) { + const QualType PT = Callee->getParamDecl(I)->getType(); + // Accept const vector<T>&, const T*, span<const T>. + if (PT->isReferenceType() && + PT.getNonReferenceType().isConstQualified()) { + Found = true; + break; + } + if (PT->isPointerType() && + PT->getPointeeType().isConstQualified()) { + Found = true; + break; + } + } + break; + } + } + if (!Found) + return false; + break; + } + + // Anything else is not read-only. + return false; + } + } + return true; +} + +void UseSpanParamCheck::registerMatchers(MatchFinder *Finder) { + // Match functions with const std::vector<T>& parameters. + Finder->addMatcher( + functionDecl( + isDefinition(), unless(isExpansionInSystemHeader()), + unless(isImplicit()), unless(isDeleted()), + has(typeLoc(forEach( + parmVarDecl(hasType(qualType(references(qualType( + isConstQualified(), + hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::vector")))))))) + .bind("param"))))) + .bind("func"), + this); +} + +void UseSpanParamCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func"); + const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param"); + assert(Func && Param && "Matcher guarantees these bindings exist"); + + // Skip if this is a virtual function (can't change signature). + if (const auto *Method = dyn_cast<CXXMethodDecl>(Func)) + if (Method->isVirtual()) + return; + + // Skip template functions for now (type deduction complexity). + if (Func->isTemplated()) + return; + + if (!allUsesAreReadOnly(Param, Func, *Result.Context)) + return; + + // Determine the element type from vector<T>. + const QualType ParamType = Param->getType().getNonReferenceType(); + const auto *Spec = + dyn_cast<ClassTemplateSpecializationDecl>(ParamType->getAsRecordDecl()); + if (!Spec || Spec->getTemplateArgs().size() < 1) + return; + + const QualType ElemType = Spec->getTemplateArgs()[0].getAsType(); + const std::string SpanType = + "std::span<const " + ElemType.getAsString() + ">"; + + auto Diag = diag(Param->getLocation(), + "parameter %0 can be changed to 'std::span'; it is only " + "used for read-only access") + << Param + << FixItHint::CreateReplacement( + Param->getTypeSourceInfo()->getTypeLoc().getSourceRange(), + SpanType); + if (auto IncludeFixIt = Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(Param->getLocation()), "<span>")) + Diag << *IncludeFixIt; +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.h b/clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.h new file mode 100644 index 0000000000000..12cfe3d395c3f --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanParamCheck.h @@ -0,0 +1,40 @@ +//===----------------------------------------------------------------------===// +// +// 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_MODERNIZE_USESPANPARAMCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANPARAMCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" + +namespace clang::tidy::modernize { + +/// Finds function parameters declared as 'const std::vector<T>&' that are only +/// used for read-only element access, and suggests using 'std::span<const T>'. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-span-param.html +class UseSpanParamCheck : public ClangTidyCheck { +public: + UseSpanParamCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } + +private: + utils::IncludeInserter Inserter; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANPARAMCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 68bab88146241..6a85fb1bee542 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -121,6 +121,13 @@ New checks ``llvm::to_vector(llvm::make_filter_range(...))`` that can be replaced with ``llvm::map_to_vector`` and ``llvm::filter_to_vector``. +- New :doc:`modernize-use-span-param + <clang-tidy/checks/modernize/use-span-param>` check. + + Finds function parameters declared as ``const std::vector<T>&`` that are only + used for read-only element access, and suggests using ``std::span<const T>`` + instead. + - New :doc:`modernize-use-string-view <clang-tidy/checks/modernize/use-string-view>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c475870ed7b31..55434a4f54b11 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -326,6 +326,7 @@ Clang-Tidy Checks :doc:`modernize-use-override <modernize/use-override>`, "Yes" :doc:`modernize-use-ranges <modernize/use-ranges>`, "Yes" :doc:`modernize-use-scoped-lock <modernize/use-scoped-lock>`, "Yes" + :doc:`modernize-use-span-param <modernize/use-span-param>`, "Yes" :doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`, "Yes" :doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes" :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-param.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-param.rst new file mode 100644 index 0000000000000..9adbb0316a6ca --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-param.rst @@ -0,0 +1,45 @@ +.. title:: clang-tidy - modernize-use-span-param + +modernize-use-span-param +======================== + +Finds function parameters declared as ``const std::vector<T>&`` that are only +used for read-only element access, and suggests using ``std::span<const T>`` +instead. + +Using ``std::span`` makes the interface more generic, allowing callers to pass +arrays, spans, or other contiguous ranges without requiring a +``std::vector``. + +For example: + +.. code-block:: c++ + + // Before + void process(const std::vector<int> &v) { + for (auto i = 0u; i < v.size(); ++i) + use(v[i]); + } + + // After + void process(std::span<const int> v) { + for (auto i = 0u; i < v.size(); ++i) + use(v[i]); + } + +The check only triggers when all uses of the parameter are read-only operations +also available on ``std::span``: + +- ``operator[]``, ``at``, ``data()``, ``size()``, ``empty()`` +- ``front()``, ``back()``, ``begin()``, ``end()`` + (and their ``c``/``r``/``cr`` variants) +- Range-based ``for`` loops +- Passing to functions accepting ``const std::vector<T>&`` or ``const T*`` + +The check does not trigger for: + +- Virtual functions (signature cannot be changed) +- Template functions +- Functions without a body (declarations only) + +This check is enabled for C++20 or later. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-span-param.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-span-param.cpp new file mode 100644 index 0000000000000..be405b64004cd --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-span-param.cpp @@ -0,0 +1,78 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-span-param %t + +namespace std { +using size_t = decltype(sizeof(0)); +template <typename T, typename Alloc = void> +class vector { +public: + using size_type = size_t; + using const_iterator = const T *; + const T &operator[](size_type i) const; + T &operator[](size_type i); + const T &at(size_type i) const; + size_type size() const; + bool empty() const; + const T *data() const; + const T &front() const; + const T &back() const; + const_iterator begin() const; + const_iterator end() const; +}; + +template <typename T> +class span {}; +} // namespace std + +// Positive: only uses operator[] and size(). +void read_size_index(const std::vector<int> &v) { + // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: parameter 'v' can be changed to 'std::span'; it is only used for read-only access [modernize-use-span-param] + for (std::size_t i = 0; i < v.size(); ++i) + (void)v[i]; +} + +// Positive: only uses data() and size(). +void read_data(const std::vector<int> &v) { + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: parameter 'v' can be changed to 'std::span'; it is only used for read-only access [modernize-use-span-param] + const int *p = v.data(); + (void)v.size(); +} + +// Positive: only uses empty(). +void read_empty(const std::vector<int> &v) { + // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: parameter 'v' can be changed to 'std::span'; it is only used for read-only access [modernize-use-span-param] + if (v.empty()) + return; +} + +// Positive: range-for loop. +void range_for(const std::vector<int> &v) { + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: parameter 'v' can be changed to 'std::span'; it is only used for read-only access [modernize-use-span-param] + for (int x : v) + (void)x; +} + +// Positive: passed to function taking const vector&. +void consumer(const std::vector<int> &); +void pass_to_const_ref(const std::vector<int> &v) { + // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: parameter 'v' can be changed to 'std::span'; it is only used for read-only access [modernize-use-span-param] + consumer(v); +} + +// Negative: non-const reference (can mutate). +void mutating(std::vector<int> &v) { + v[0]; +} + +// Negative: virtual method (can't change signature). +struct Base { + virtual void process(const std::vector<int> &v); +}; + +// Negative: template function. +template <typename T> +void templated(const std::vector<T> &v) { + (void)v.size(); +} + +// Negative: no body (declaration only). +void no_body(const std::vector<int> &v); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
