llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Helmut Januschka (hjanuschka) <details> <summary>Changes</summary> Add new clang-tidy check that suggests using std::span's more expressive first() and last() member functions instead of equivalent subspan() calls. The check modernizes code by replacing: - subspan(0, n) -> first(n) - subspan(n) -> last(size() - n) These dedicated methods were added to the standard 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: ```cpp std::span<int> s = ...; auto sub1 = s.subspan(0, n); // transforms to: auto sub1 = s.first(n); auto sub2 = s.subspan(n); // transforms to: auto sub2 = s.last(s.size() - n); auto sub3 = s.subspan(1, n); // not transformed, no direct equivalent ``` not sure if this is a readability or a modernize check ❓ --- Full diff: https://github.com/llvm/llvm-project/pull/118074.diff 7 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp (+3) - (added) clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp (+98) - (added) clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h (+40) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst (+19) - (added) clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp (+50) ``````````diff diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index c919d49b42873a..47dd12a2640b6c 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -49,6 +49,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp + UseSpanFirstLastCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 18607593320635..6fc5de5aad20b7 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -42,6 +42,7 @@ #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" #include "UseRangesCheck.h" +#include "UseSpanFirstLastCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" @@ -122,6 +123,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); + CheckFactories.registerCheck<UseSpanFirstLastCheck>("modernize-use-span-first-last"); + } }; diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp new file mode 100644 index 00000000000000..6cf01386b0c3fb --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp @@ -0,0 +1,98 @@ +//===--- 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/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +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) { + // Get arguments + 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; + bool IsZeroOffset = false; + + // Check if offset is zero through any implicit casts + const Expr *OffsetE = Offset->IgnoreImpCasts(); + if (const auto *IL = dyn_cast<IntegerLiteral>(OffsetE)) { + IsZeroOffset = IL->getValue() == 0; + } + + // Build replacement text + std::string Replacement; + if (IsZeroOffset && Count) { + // subspan(0, count) -> first(count) + auto CountStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(Count->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + const auto *Base = + cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); + auto BaseStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(Base->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + Replacement = BaseStr.str() + ".first(" + CountStr.str() + ")"; + } else if (NumArgs == 1) { + // subspan(n) -> last(size() - n) + auto OffsetStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(Offset->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + + const auto *Base = + cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); + auto BaseStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(Base->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + + Replacement = BaseStr.str() + ".last(" + BaseStr.str() + ".size() - " + + OffsetStr.str() + ")"; + } + + if (!Replacement.empty()) { + if (IsZeroOffset && Count) { + diag(Call->getBeginLoc(), "prefer span::first() over subspan()") + << FixItHint::CreateReplacement(Call->getSourceRange(), Replacement); + } else { + diag(Call->getBeginLoc(), "prefer span::last() over subspan()") + << FixItHint::CreateReplacement(Call->getSourceRange(), Replacement); + } + } +} + +} // namespace clang::tidy::modernize \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h new file mode 100644 index 00000000000000..8d4c6035f7ec76 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h @@ -0,0 +1,40 @@ +//===--- 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_MODERNIZE_USESPANFIRSTLASTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANFIRSTLASTCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Converts std::span::subspan() calls to the more modern first()/last() +/// methods where applicable. +/// +/// For example: +/// \code +/// std::span<int> s = ...; +/// auto sub = s.subspan(0, n); // -> auto sub = s.first(n); +/// auto sub2 = s.subspan(n); // -> auto sub2 = s.last(s.size() - n); +/// \endcode +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; + +private: + void handleSubspanCall(const ast_matchers::MatchFinder::MatchResult &Result, + const CXXMemberCallExpr *Call); +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANFIRSTLASTCHECK_H \ No newline at end of file diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f050391110385e..04a45d002c0d1d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -145,6 +145,10 @@ New checks New check aliases ^^^^^^^^^^^^^^^^^ +- New check `modernize-use-span-first-last` has been added that suggests using + ``std::span::first()`` and ``std::span::last()`` member functions instead of + equivalent ``subspan()``. + - New alias :doc:`cert-arr39-c <clang-tidy/checks/cert/arr39-c>` to :doc:`bugprone-sizeof-expression <clang-tidy/checks/bugprone/sizeof-expression>` was added. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst new file mode 100644 index 00000000000000..e8aad59bb2264f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - modernize-use-span-first-last + +modernize-use-span-first-last +============================ + +Checks for uses of ``std::span::subspan()`` that can be replaced with clearer +``first()`` or ``last()`` member functions. + +Covered scenarios: + +==================================== ================================== +Expression Replacement +------------------------------------ ---------------------------------- +``s.subspan(0, n)`` ``s.first(n)`` +``s.subspan(n)`` ``s.last(s.size() - n)`` +==================================== ================================== + +Non-zero offset with count (like ``subspan(1, n)``) has no direct equivalent +using ``first()`` or ``last()``, so these cases are not transformed. \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp new file mode 100644 index 00000000000000..cb78bc02f22d4f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp @@ -0,0 +1,50 @@ +// RUN: %check_clang_tidy %s modernize-use-span-first-last %t + +namespace std { +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; } +}; +} // namespace std + +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(2); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer span::last() over subspan() + // CHECK-FIXES: auto sub2 = s.last(s.size() - 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 +} \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/118074 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
