From 1fcd5fe0cd378555b8b27a5b452006b14dac4788 Mon Sep 17 00:00:00 2001 From: JJ Marr Date: Wed, 14 May 2025 20:43:39 -0400 Subject: [PATCH] Add modernize-use-span This linter check recommends using std::span over const references to std::array or std::vector. --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../clang-tidy/modernize/UseSpanCheck.cpp | 142 ++++++++++++++++++ .../clang-tidy/modernize/UseSpanCheck.h | 39 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../clang-tidy/checks/modernize/use-span.rst | 48 ++++++ .../checkers/modernize/use-span.cpp | 131 ++++++++++++++++ 8 files changed, 371 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseSpanCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseSpanCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-span.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-span.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index bab1167fb15ff..9d6507ddc5090 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -42,6 +42,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseNullptrCheck.cpp UseOverrideCheck.cpp UseRangesCheck.cpp + UseSpanCheck.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 fc46c72982fdc..1fba227a7d63b 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -43,6 +43,7 @@ #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" #include "UseRangesCheck.h" +#include "UseSpanCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" @@ -80,6 +81,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck( "modernize-use-integer-sign-comparison"); CheckFactories.registerCheck("modernize-use-ranges"); + CheckFactories.registerCheck( + "modernize-use-span"); CheckFactories.registerCheck( "modernize-use-starts-ends-with"); CheckFactories.registerCheck("modernize-use-std-format"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseSpanCheck.cpp new file mode 100644 index 0000000000000..9a9f3c3fd9f84 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanCheck.cpp @@ -0,0 +1,142 @@ +//===--- UseSpanCheck.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 "UseSpanCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "../utils/IncludeInserter.h" +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +namespace { +AST_MATCHER(QualType, isRefToVectorOrArray) { + if (!Node->isReferenceType()) + return false; + + QualType PointeeType = Node->getPointeeType(); + + const Type *UnqualifiedType = PointeeType.getTypePtr()->getUnqualifiedDesugaredType(); + if (!UnqualifiedType || !UnqualifiedType->isRecordType()) + return false; + + const CXXRecordDecl *Record = UnqualifiedType->getAsCXXRecordDecl(); + if (!Record) + return false; + + const std::string Name = Record->getQualifiedNameAsString(); + return Name == "std::vector" || Name == "std::array"; +} +} // namespace + +UseSpanCheck::UseSpanCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Inserter(utils::IncludeSorter::IS_LLVM, areDiagsSelfContained()) {} + +void UseSpanCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +void UseSpanCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl( + forEachDescendant( + parmVarDecl(hasType(qualType(isRefToVectorOrArray()))) + .bind("param"))), + this); +} + +void UseSpanCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Param = Result.Nodes.getNodeAs("param"); + if (!Param) + return; + + QualType ParamType = Param->getType(); + if (!ParamType->isReferenceType()) + return; + + // Get the pointee type (the vector/array type) + QualType PointeeType = ParamType->getPointeeType(); + bool IsConst = PointeeType.isConstQualified(); + + const Type *UnqualifiedType = PointeeType.getTypePtr()->getUnqualifiedDesugaredType(); + if (!UnqualifiedType || !UnqualifiedType->isRecordType()) + return; + + const CXXRecordDecl *Record = UnqualifiedType->getAsCXXRecordDecl(); + if (!Record) + return; + + const std::string RecordName = Record->getQualifiedNameAsString(); + if (RecordName != "std::vector" && RecordName != "std::array") + return; + + // Check if it's a template specialization + if (!isa(Record)) + return; + + // Get the template arguments + const auto *TemplateSpecRecord = cast(Record); + const TemplateArgumentList &Args = TemplateSpecRecord->getTemplateArgs(); + if (Args.size() < 1) + return; + + // Get the element type from the first template argument + const TemplateArgument &Arg = Args[0]; + if (Arg.getKind() != TemplateArgument::Type) + return; + + QualType ElementType = Arg.getAsType(); + + // Get the source range for the parameter type + TypeSourceInfo *TSI = Param->getTypeSourceInfo(); + TypeLoc TL = TSI->getTypeLoc(); + + // Get the source range for the entire type, including qualifiers + SourceRange TypeRange = TL.getSourceRange(); + + // Create the diagnostic + auto Diag = diag(Param->getBeginLoc(), + "parameter %0 is reference to %1; consider using std::span instead") + << Param + << (RecordName == "std::vector" ? "std::vector" : "std::array"); + + // Create the fix-it hint + std::string SpanType; + std::string ElementTypeWithConst = IsConst ? "const " + ElementType.getAsString() : ElementType.getAsString(); + + // For std::array, we should preserve the size in the span + if (RecordName == "std::array" && Args.size() >= 2) { + const TemplateArgument &SizeArg = Args[1]; + if (SizeArg.getKind() == TemplateArgument::Integral) { + llvm::APSInt Size = SizeArg.getAsIntegral(); + // Convert APSInt to string + SmallString<16> SizeStr; + Size.toString(SizeStr, 10); + SpanType = "std::span<" + ElementTypeWithConst + ", " + SizeStr.str().str() + ">"; + } else { + SpanType = "std::span<" + ElementTypeWithConst + ">"; + } + } else { + SpanType = "std::span<" + ElementTypeWithConst + ">"; + } + + // Create the replacement for the entire type including qualifiers + Diag << FixItHint::CreateReplacement(TypeRange, SpanType); + + // Add the include for + Diag << Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(Param->getBeginLoc()), + ""); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanCheck.h b/clang-tools-extra/clang-tidy/modernize/UseSpanCheck.h new file mode 100644 index 0000000000000..a70cadf1e5201 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanCheck.h @@ -0,0 +1,39 @@ +//===--- UseSpanCheck.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_MODERNIZE_USESPANCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" + +namespace clang::tidy::modernize { + +/// Suggests replacing const references to std::vector and std::array with +/// std::span. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-span.html +class UseSpanCheck : public ClangTidyCheck { +public: + UseSpanCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) 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_USESPANCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 579fca54924d5..aab6396537121 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -136,6 +136,12 @@ New checks Finds potentially erroneous calls to ``reset`` method on smart pointers when the pointee type also has a ``reset`` method. +- New :doc:`modernize-use-span + ` check. + + Suggests replacing const references to std::vector and std::array with + std::span. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 18f1467285fab..01f6a11dab9c5 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -310,6 +310,7 @@ Clang-Tidy Checks :doc:`modernize-use-nullptr `, "Yes" :doc:`modernize-use-override `, "Yes" :doc:`modernize-use-ranges `, "Yes" + :doc:`modernize-use-span `, "Yes" :doc:`modernize-use-starts-ends-with `, "Yes" :doc:`modernize-use-std-format `, "Yes" :doc:`modernize-use-std-numbers `, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span.rst new file mode 100644 index 0000000000000..9f599a2e86b32 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span.rst @@ -0,0 +1,48 @@ +.. title:: clang-tidy - modernize-use-span + +modernize-use-span +================== + +This check suggests using ``std::span`` in function parameters when taking a +range of contiguous elements by reference. ``std::span`` can accept vectors, +arrays, and C-style arrays, so the function is no longer bound to a specific +container type. + +This check implements `R.14`` +from the C++ Core Guidelines. + +This check requires C++20 or later. + +Examples +-------- + +.. code-block:: c++ + + // Before + void process(const std::vector& vec) { + for (const auto& val : vec) { + // Process val + } + } + + void analyze(const std::array& arr) { + for (const auto& val : arr) { + // Analyze val + } + } + + // After + void process(std::span vec) { + for (const auto& val : vec) { + // Process val + } + } + + void analyze(std::span arr) { + for (const auto& val : arr) { + // Analyze val + } + } + +The transformed code can now accept any contiguous container of the appropriate +element type and optionally specified length. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-span.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-span.cpp new file mode 100644 index 0000000000000..123f31d7c3046 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-span.cpp @@ -0,0 +1,131 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-span %t + +// Mock std::vector and std::array for testing +using size_t = unsigned long; +using ptrdiff_t = long; + +namespace std { +template +struct remove_cv { using type = T; }; +template +using remove_cv_t = typename remove_cv::type; + +template +class reverse_iterator {}; + +template +class vector { +public: + using value_type = T; + using iterator = T*; + using const_iterator = const T*; + const_iterator begin() const { return nullptr; } + const_iterator end() const { return nullptr; } +}; + +template +class array { +public: + using value_type = T; + using iterator = T*; + using const_iterator = const T*; + const_iterator begin() const { return nullptr; } + const_iterator end() const { return nullptr; } +}; + +template +class span { +public: + using element_type = T; + using value_type = std::remove_cv_t; + using size_type = size_t; + using difference_type = ptrdiff_t; + using pointer = T*; + using const_pointer = const T*; + using reference = T&; + using const_reference = const T&; + using iterator = pointer; + using reverse_iterator = std::reverse_iterator; +}; +} // namespace std + +// Test cases that should trigger the check + +// Case 1: Function with const reference to std::vector +void processVector(const std::vector& vec) { +// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: parameter 'vec' is const reference to std::vector; consider using std::span instead [modernize-use-span] +// CHECK-FIXES: void processVector(std::span vec) { + int sum = 0; + for (const auto& val : vec) { + sum += val; + } +} + +// Case 2: Function with const reference to std::array +void processArray(const std::array& arr) { +// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'arr' is const reference to std::array; consider using std::span instead [modernize-use-span] +// CHECK-FIXES: void processArray(std::span arr) { + double sum = 0.0; + for (const auto& val : arr) { + sum += val; + } +} + +// Case 3: Method with const reference to std::vector +class DataProcessor { +public: + void process(const std::vector& data) { + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: parameter 'data' is const reference to std::vector; consider using std::span instead [modernize-use-span] + // CHECK-FIXES: void process(std::span data) { + // Process data + } +}; + +// Case 4: Function with multiple parameters including const reference to std::vector +void multiParam(int a, const std::vector& chars, double b) { +// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: parameter 'chars' is const reference to std::vector; consider using std::span instead [modernize-use-span] +// CHECK-FIXES: void multiParam(int a, std::span chars, double b) { + // Process chars +} + +// Case 5: Non-const reference to std::vector that modifies an element +void modifyVector(std::vector& vec) { +// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: parameter 'vec' is reference to std::vector; consider using std::span instead [modernize-use-span] +// CHECK-FIXES: void modifyVector(std::span vec) { + vec[42] = 1; +} + +// Case 5b: Non-const reference to std::array +void modifyArray(std::array& arr) { +// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'arr' is reference to std::array; consider using std::span instead [modernize-use-span] +// CHECK-FIXES: void modifyArray(std::span arr) { + // Modify arr +} + +// Test cases that should NOT trigger the check + +// Case 6: Const reference to a different container +void processOtherContainer(const int& str) { + // Process str +} + +// Case 7: Value parameter +void copyVector(std::vector vec) { + // Process vec +} + +// Case 8: Const value parameter +void constCopyVector(const std::vector vec) { + // Process vec +} + +// Case 9: Function with rvalue reference +void moveVector(std::vector&& vec) { + // Move from vec +} + +// Case 10: Template function +template +void processTemplate(const std::vector& vec) { + // Process vec +}