https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/118074
>From 39e01dbbf4941f86f7b635a2d26b976e1ee0ed89 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <[email protected]> Date: Wed, 18 Feb 2026 08:58:31 +0100 Subject: [PATCH] [clang-tidy] Add readability-use-span-first-last check Add new clang-tidy check that suggests using std::span's more expressive first() and last() member functions instead of equivalent subspan() calls. These dedicated methods were added to C++20 to provide clearer alternatives to common subspan operations. They improve readability by better expressing intent and are less error-prone by eliminating manual offset calculations. For example: s.subspan(0, n) -> s.first(n) s.subspan(s.size() - n) -> s.last(n) Non-zero offset with count (like subspan(1, n)) or offset-only calls (like subspan(n)) are not transformed as they have no clearer equivalent. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../readability/UseSpanFirstLastCheck.cpp | 93 ++++++++++ .../readability/UseSpanFirstLastCheck.h | 43 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../readability/use-span-first-last.rst | 24 +++ .../readability/use-span-first-last.cpp | 171 ++++++++++++++++++ 8 files changed, 342 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index f1f3cde32feff..941416160cb21 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -63,6 +63,7 @@ add_clang_library(clangTidyReadabilityModule STATIC UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp UseConcisePreprocessorDirectivesCheck.cpp + UseSpanFirstLastCheck.cpp UseStdMinMaxCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index c582dc98eac6b..f213c02dd9fee 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -65,6 +65,7 @@ #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" #include "UseConcisePreprocessorDirectivesCheck.h" +#include "UseSpanFirstLastCheck.h" #include "UseStdMinMaxCheck.h" namespace clang::tidy { @@ -188,6 +189,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-use-anyofallof"); CheckFactories.registerCheck<UseConcisePreprocessorDirectivesCheck>( "readability-use-concise-preprocessor-directives"); + CheckFactories.registerCheck<UseSpanFirstLastCheck>( + "readability-use-span-first-last"); CheckFactories.registerCheck<UseStdMinMaxCheck>( "readability-use-std-min-max"); } diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp new file mode 100644 index 0000000000000..16e56ec45607c --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -0,0 +1,93 @@ +//===--- 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 "../utils/Matchers.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; +using namespace clang::tidy::matchers; + +namespace clang::tidy::readability { + +void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) { + const auto HasSpanType = + hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( + classTemplateSpecializationDecl(hasName("::std::span")))))); + + // Match span.subspan(0, n) -> first(n) + Finder->addMatcher( + cxxMemberCallExpr( + argumentCountIs(2), + callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("subspan"))))), + on(expr(HasSpanType).bind("span_object")), + hasArgument(0, integerLiteral(equals(0))), + hasArgument(1, expr().bind("count"))) + .bind("first_subspan"), + this); + + // Match span.subspan(span.size() - n) or span.subspan(std::ranges::size(span) + // - n) + // -> last(n) + const auto SizeCall = anyOf( + cxxMemberCallExpr( + callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("size"))))), + on(expr(isStatementIdenticalToBoundNode("span_object")))), + callExpr(callee(functionDecl( + hasAnyName("::std::size", "::std::ranges::size"))), + hasArgument( + 0, expr(isStatementIdenticalToBoundNode("span_object"))))); + + Finder->addMatcher( + cxxMemberCallExpr( + argumentCountIs(1), + callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("subspan"))))), + on(expr(HasSpanType).bind("span_object")), + hasArgument(0, binaryOperator(hasOperatorName("-"), hasLHS(SizeCall), + hasRHS(expr().bind("count"))))) + .bind("last_subspan"), + this); +} + +void UseSpanFirstLastCheck::check(const MatchFinder::MatchResult &Result) { + const auto *SpanObj = Result.Nodes.getNodeAs<Expr>("span_object"); + if (!SpanObj) + return; + + const auto *SubSpan = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("first_subspan"); + bool IsFirst = true; + if (!SubSpan) { + SubSpan = Result.Nodes.getNodeAs<CXXMemberCallExpr>("last_subspan"); + IsFirst = false; + } + + if (!SubSpan) + return; + + const auto *Count = Result.Nodes.getNodeAs<Expr>("count"); + assert(Count && "Count expression must exist due to AST matcher"); + + const StringRef CountText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Count->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + const StringRef SpanText = Lexer::getSourceText( + CharSourceRange::getTokenRange(SpanObj->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + const StringRef FirstOrLast = IsFirst ? "first" : "last"; + const std::string Replacement = + (Twine(SpanText) + "." + FirstOrLast + "(" + CountText + ")").str(); + + diag(SubSpan->getBeginLoc(), "prefer 'span::%0()' over 'subspan()'") + << FirstOrLast + << FixItHint::CreateReplacement(SubSpan->getSourceRange(), Replacement); +} +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h new file mode 100644 index 0000000000000..69036df9f0706 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -0,0 +1,43 @@ +//===--- UseSpanFirstLastCheck.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_READABILITY_USESPANFIRSTLASTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USESPANFIRSTLASTCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Suggests using clearer 'std::span' member functions 'first()'/'last()' +/// instead of equivalent 'subspan()' calls where applicable. +/// +/// For example: +/// \code +/// std::span<int> s = ...; +/// auto sub1 = s.subspan(0, n); // -> auto sub1 = s.first(n); +/// auto sub2 = s.subspan(s.size() - n); // -> auto sub2 = s.last(n); +/// auto sub3 = s.subspan(1, n); // not changed +/// auto sub4 = s.subspan(n); // not changed +/// \endcode +/// +/// The check is only active in C++20 mode. +class UseSpanFirstLastCheck : public ClangTidyCheck { +public: + UseSpanFirstLastCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USESPANFIRSTLASTCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2bb4800df47c9..e6437a333c124 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -139,6 +139,12 @@ New checks Checks for presence or absence of trailing commas in enum definitions and initializer lists. +- New :doc:`readability-use-span-first-last + <clang-tidy/checks/readability/use-span-first-last>` check. + + Suggests using ``std::span::first()`` and ``std::span::last()`` member + functions instead of equivalent ``subspan()`` calls. + 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 0eabd9929dc39..dcef5fbfd775c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -430,6 +430,7 @@ Clang-Tidy Checks :doc:`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix>`, "Yes" :doc:`readability-use-anyofallof <readability/use-anyofallof>`, :doc:`readability-use-concise-preprocessor-directives <readability/use-concise-preprocessor-directives>`, "Yes" + :doc:`readability-use-span-first-last <readability/use-span-first-last>`, "Yes" :doc:`readability-use-std-min-max <readability/use-std-min-max>`, "Yes" :doc:`zircon-temporary-objects <zircon/temporary-objects>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst new file mode 100644 index 0000000000000..07b0c61765980 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst @@ -0,0 +1,24 @@ +.. title:: clang-tidy - readability-use-span-first-last + +readability-use-span-first-last +=============================== + +Suggests using ``std::span::first()`` and ``std::span::last()`` member +functions instead of equivalent ``subspan()`` calls. These dedicated methods +were added to C++20 to provide more expressive alternatives to common subspan +operations. + +Covered scenarios: + +=============================== ================ +Expression Replacement +------------------------------- ---------------- +``s.subspan(0, n)`` ``s.first(n)`` +``s.subspan(s.size() - n)`` ``s.last(n)`` +=============================== ================ + +Non-zero offset with count (like ``subspan(1, n)``) or offset-only calls +(like ``subspan(n)``) have no clearer equivalent using ``first()`` or +``last()``, so these cases are not transformed. + +This check is only active when C++20 or later is used. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp new file mode 100644 index 0000000000000..3c8701af4e38f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp @@ -0,0 +1,171 @@ +// RUN: %check_clang_tidy -std=c++20 %s readability-use-span-first-last %t + +namespace std { + +enum class byte : unsigned char {}; + +template <typename T> +class span { + T* ptr; + __SIZE_TYPE__ len; + +public: + span(T* p, __SIZE_TYPE__ l) : ptr(p), len(l) {} + + span<T> subspan(__SIZE_TYPE__ offset) const { + return span(ptr + offset, len - offset); + } + + span<T> subspan(__SIZE_TYPE__ offset, __SIZE_TYPE__ count) const { + return span(ptr + offset, count); + } + + span<T> first(__SIZE_TYPE__ count) const { + return span(ptr, count); + } + + span<T> last(__SIZE_TYPE__ count) const { + return span(ptr + (len - count), count); + } + + __SIZE_TYPE__ size() const { return len; } + __SIZE_TYPE__ size_bytes() const { return len * sizeof(T); } +}; +} // namespace std + +// Add here, right after the std namespace closes: +namespace std::ranges { + template<typename T> + __SIZE_TYPE__ size(const span<T>& s) { return s.size(); } +} + +void test() { + int arr[] = {1, 2, 3, 4, 5}; + std::span<int> s(arr, 5); + + auto sub1 = s.subspan(0, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 'subspan()' + // CHECK-FIXES: auto sub1 = s.first(3); + + auto sub2 = s.subspan(s.size() - 2); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub2 = s.last(2); + + __SIZE_TYPE__ n = 2; + auto sub3 = s.subspan(0, n); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 'subspan()' + // CHECK-FIXES: auto sub3 = s.first(n); + + auto sub4 = s.subspan(1, 2); // No warning + auto sub5 = s.subspan(2); // No warning + + +#define ZERO 0 +#define TWO 2 +#define SIZE_MINUS(s, n) s.size() - n +#define MAKE_SUBSPAN(obj, n) obj.subspan(0, n) +#define MAKE_LAST_N(obj, n) obj.subspan(obj.size() - n) + + auto sub6 = s.subspan(SIZE_MINUS(s, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub6 = s.last(2); + + auto sub7 = MAKE_SUBSPAN(s, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: prefer 'span::first()' over 'subspan()' + // CHECK-FIXES: auto sub7 = s.first(3); + + auto sub8 = MAKE_LAST_N(s, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub8 = s.last(2); + +} + +template <typename T> +void testTemplate() { + T arr[] = {1, 2, 3, 4, 5}; + std::span<T> s(arr, 5); + + auto sub1 = s.subspan(0, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 'subspan()' + // CHECK-FIXES: auto sub1 = s.first(3); + + auto sub2 = s.subspan(s.size() - 2); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub2 = s.last(2); + + __SIZE_TYPE__ n = 2; + auto sub3 = s.subspan(0, n); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 'subspan()' + // CHECK-FIXES: auto sub3 = s.first(n); + + auto sub4 = s.subspan(1, 2); // No warning + auto sub5 = s.subspan(2); // No warning + + auto complex = s.subspan(0 + (s.size() - 2), 3); // No warning + + auto complex2 = s.subspan(100 + (s.size() - 2)); // No warning +} + +// Test instantiation +void testInt() { + testTemplate<int>(); +} + +void test_ranges() { + int arr[] = {1, 2, 3, 4, 5}; + std::span<int> s(arr, 5); + + auto sub1 = s.subspan(std::ranges::size(s) - 2); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub1 = s.last(2); + + __SIZE_TYPE__ n = 2; + auto sub2 = s.subspan(std::ranges::size(s) - n); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub2 = s.last(n); +} + +void test_different_spans() { + int arr1[] = {1, 2, 3, 4, 5}; + int arr2[] = {6, 7, 8, 9, 10}; + std::span<int> s1(arr1, 5); + std::span<int> s2(arr2, 5); + + // These should NOT trigger warnings as they use size() from a different span + auto sub1 = s1.subspan(s2.size() - 2); // No warning + auto sub2 = s2.subspan(s1.size() - 3); // No warning + + // Also check with std::ranges::size + auto sub3 = s1.subspan(std::ranges::size(s2) - 2); // No warning + auto sub4 = s2.subspan(std::ranges::size(s1) - 3); // No warning + + // Mixed usage should also not trigger + auto sub5 = s1.subspan(s2.size() - s1.size()); // No warning + + // Verify that correct usage still triggers warnings + auto good1 = s1.subspan(s1.size() - 2); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto good1 = s1.last(2); + + auto good2 = s2.subspan(std::ranges::size(s2) - 3); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto good2 = s2.last(3); +} + +void test_span_of_bytes() { + std::byte arr[] = {std::byte{0x1}, std::byte{0x2}, std::byte{0x3}, + std::byte{0x4}, std::byte{0x5}}; + std::span<std::byte> s(arr, 5); + + auto sub1 = s.subspan(0, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 'subspan()' + // CHECK-FIXES: auto sub1 = s.first(3); + + auto sub2 = s.subspan(s.size() - 2); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 'subspan()' + // CHECK-FIXES: auto sub2 = s.last(2); + + // size_bytes() is not the same as size() in general, so should not trigger + auto sub3 = s.subspan(s.size_bytes() - 2); // No warning +} + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
