diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 8f303c51e1b0d..fa243fbf740cc 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -57,6 +57,7 @@ add_clang_library(clangTidyReadabilityModule STATIC UniqueptrDeleteReleaseCheck.cpp UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.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 d61c0ba39658e..6fc163c1dabb5 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -60,6 +60,7 @@ #include "UniqueptrDeleteReleaseCheck.h" #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" +#include "UseSpanFirstLastCheck.h" #include "UseStdMinMaxCheck.h" namespace clang::tidy { @@ -170,6 +171,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-uppercase-literal-suffix"); CheckFactories.registerCheck( "readability-use-anyofallof"); + CheckFactories.registerCheck( + "readability-use-span-first-last"); CheckFactories.registerCheck( "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..1db0ba54266e3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -0,0 +1,97 @@ +//===--- 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")))))); + + const auto SubspanDecl = cxxMethodDecl( + hasName("subspan"), + ofClass(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("span_object"); + if (!SpanObj) + return; + + const auto *SubSpan = + Result.Nodes.getNodeAs("first_subspan"); + bool IsFirst = true; + if (!SubSpan) { + SubSpan = Result.Nodes.getNodeAs("last_subspan"); + IsFirst = false; + } + + if (!SubSpan) + return; + + const auto *Count = Result.Nodes.getNodeAs("count"); + assert(Count && "Count expression must exist due to AST matcher"); + + StringRef CountText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Count->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + StringRef SpanText = Lexer::getSourceText( + CharSourceRange::getTokenRange(SpanObj->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + const StringRef FirstOrLast = IsFirst ? "first" : "last"; + 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 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 f050391110385..3f032d6f0b359 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -142,6 +142,12 @@ New checks Finds cases when an uninstantiated virtual member function in a template class causes cross-compiler incompatibility. +- New :doc:`readability-use-span-first-last + ` check. + + Suggests using ``std::span::first()`` and ``std::span::last()`` member functions instead of + equivalent ``subspan()``. + New check aliases ^^^^^^^^^^^^^^^^^ 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..a73e2d6fd1090 --- /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()``. 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..e8d6f7c29c3a0 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp @@ -0,0 +1,150 @@ +// RUN: %check_clang_tidy -std=c++20 %s readability-use-span-first-last %t + +namespace std { +template +class span { + T* ptr; + __SIZE_TYPE__ len; + +public: + span(T* p, __SIZE_TYPE__ l) : ptr(p), len(l) {} + + span subspan(__SIZE_TYPE__ offset) const { + return span(ptr + offset, len - offset); + } + + span subspan(__SIZE_TYPE__ offset, __SIZE_TYPE__ count) const { + return span(ptr + offset, count); + } + + span first(__SIZE_TYPE__ count) const { + return span(ptr, count); + } + + span last(__SIZE_TYPE__ count) const { + return span(ptr + (len - count), count); + } + + __SIZE_TYPE__ size() const { return len; } +}; +} // namespace std + +// Add here, right after the std namespace closes: +namespace std::ranges { + template + __SIZE_TYPE__ size(const span& s) { return s.size(); } +} + +void test() { + int arr[] = {1, 2, 3, 4, 5}; + std::span 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 +void testTemplate() { + T arr[] = {1, 2, 3, 4, 5}; + std::span 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(); +} + +void test_ranges() { + int arr[] = {1, 2, 3, 4, 5}; + std::span 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 s1(arr1, 5); + std::span 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); +} +