https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/118074
>From cb748c34d35b8c0c9ca93a67b111dcf5d7665b34 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:17:49 +0100 Subject: [PATCH 01/23] [clang-tidy] Add modernize-use-span-first-last check Add new check that modernizes std::span::subspan() calls to use the more expressive first() and last() member functions where applicable. --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../modernize/UseSpanFirstLastCheck.cpp | 97 +++++++++++++++++++ .../modernize/UseSpanFirstLastCheck.h | 40 ++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/modernize/use-span-first-last.rst | 19 ++++ .../modernize-subspan-conversion.cpp | 50 ++++++++++ 7 files changed, 214 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp 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..f57571f2aa7c86 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/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 "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..141b848be9abbb --- /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 >From ec5a6b0ceb0fbbf03ea36a38fb627b25ab4e62de Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:19:21 +0100 Subject: [PATCH 02/23] format --- .../modernize/UseSpanFirstLastCheck.cpp | 41 ++++++++++--------- .../modernize/UseSpanFirstLastCheck.h | 6 +-- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp index f57571f2aa7c86..6cf01386b0c3fb 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp @@ -18,17 +18,15 @@ namespace clang::tidy::modernize { void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) { // Match span::subspan calls - const auto HasSpanType = hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(classTemplateSpecializationDecl( - hasName("::std::span")))))); + 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); + Finder->addMatcher(cxxMemberCallExpr(callee(memberExpr(hasDeclaration( + cxxMethodDecl(hasName("subspan"))))), + on(expr(HasSpanType))) + .bind("subspan"), + this); } void UseSpanFirstLastCheck::check(const MatchFinder::MatchResult &Result) { @@ -52,7 +50,7 @@ void UseSpanFirstLastCheck::handleSubspanCall( bool IsZeroOffset = false; // Check if offset is zero through any implicit casts - const Expr* OffsetE = Offset->IgnoreImpCasts(); + const Expr *OffsetE = Offset->IgnoreImpCasts(); if (const auto *IL = dyn_cast<IntegerLiteral>(OffsetE)) { IsZeroOffset = IL->getValue() == 0; } @@ -64,7 +62,8 @@ void UseSpanFirstLastCheck::handleSubspanCall( auto CountStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Count->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); - const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); + const auto *Base = + cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); auto BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); @@ -74,22 +73,24 @@ void UseSpanFirstLastCheck::handleSubspanCall( auto OffsetStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Offset->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); - - const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); + + 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() + ")"; + + 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); + 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); + diag(Call->getBeginLoc(), "prefer span::last() over subspan()") + << FixItHint::CreateReplacement(Call->getSourceRange(), Replacement); } } } diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h index 141b848be9abbb..8d4c6035f7ec76 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h @@ -13,8 +13,8 @@ namespace clang::tidy::modernize { -/// Converts std::span::subspan() calls to the more modern first()/last() methods -/// where applicable. +/// Converts std::span::subspan() calls to the more modern first()/last() +/// methods where applicable. /// /// For example: /// \code @@ -32,7 +32,7 @@ class UseSpanFirstLastCheck : public ClangTidyCheck { private: void handleSubspanCall(const ast_matchers::MatchFinder::MatchResult &Result, - const CXXMemberCallExpr *Call); + const CXXMemberCallExpr *Call); }; } // namespace clang::tidy::modernize >From 5cf1b7ce1fcaf87a26f1ad1ff0d265a47613c144 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:28:42 +0100 Subject: [PATCH 03/23] format --- clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 6fc5de5aad20b7..c473c80e3cd0eb 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -123,7 +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"); + CheckFactories.registerCheck<UseSpanFirstLastCheck>( + "modernize-use-span-first-last"); } }; >From b357f8c8fe607924ba6c5537f42ebfffaa09c011 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:36:31 +0100 Subject: [PATCH 04/23] format --- clang-tools-extra/clang-tidy/modernize/CMakeLists.txt | 1 - .../clang-tidy/modernize/ModernizeTidyModule.cpp | 3 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../clang-tidy/readability/ReadabilityTidyModule.cpp | 3 +++ .../UseSpanFirstLastCheck.cpp | 4 ++-- .../{modernize => readability}/UseSpanFirstLastCheck.h | 10 +++++----- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../use-starts-ends-with.rst | 4 ++-- .../use-span-first-last.cpp} | 2 +- 9 files changed, 15 insertions(+), 15 deletions(-) rename clang-tools-extra/clang-tidy/{modernize => readability}/UseSpanFirstLastCheck.cpp (97%) rename clang-tools-extra/clang-tidy/{modernize => readability}/UseSpanFirstLastCheck.h (79%) rename clang-tools-extra/docs/clang-tidy/checks/{modernize => readability}/use-starts-ends-with.rst (92%) rename clang-tools-extra/test/clang-tidy/checkers/{modernize/modernize-subspan-conversion.cpp => readability/use-span-first-last.cpp} (95%) diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 47dd12a2640b6c..c919d49b42873a 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -49,7 +49,6 @@ 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 c473c80e3cd0eb..b2a8f9dd20adba 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -42,7 +42,6 @@ #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" #include "UseRangesCheck.h" -#include "UseSpanFirstLastCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" @@ -123,8 +122,6 @@ 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/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 8f303c51e1b0da..f9f9e8e7f19685 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -58,6 +58,7 @@ add_clang_library(clangTidyReadabilityModule STATIC UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp UseStdMinMaxCheck.cpp + UseSpanFirstLastCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index d61c0ba39658e5..9729d080f63a84 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 { @@ -172,6 +173,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-use-anyofallof"); CheckFactories.registerCheck<UseStdMinMaxCheck>( "readability-use-std-min-max"); + CheckFactories.registerCheck<UseSpanFirstLastCheck>( + "readability-use-span-first-last"); } }; diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp similarity index 97% rename from clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp rename to clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index 6cf01386b0c3fb..da7d147565fbdf 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -14,7 +14,7 @@ using namespace clang::ast_matchers; -namespace clang::tidy::modernize { +namespace clang::tidy::readability { void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) { // Match span::subspan calls @@ -95,4 +95,4 @@ void UseSpanFirstLastCheck::handleSubspanCall( } } -} // namespace clang::tidy::modernize \ No newline at end of file +} // namespace clang::tidy::readability \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h similarity index 79% rename from clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h rename to clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h index 8d4c6035f7ec76..271730ed4985a3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -6,12 +6,12 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANFIRSTLASTCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANFIRSTLASTCHECK_H +#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::modernize { +namespace clang::tidy::readability { /// Converts std::span::subspan() calls to the more modern first()/last() /// methods where applicable. @@ -35,6 +35,6 @@ class UseSpanFirstLastCheck : public ClangTidyCheck { const CXXMemberCallExpr *Call); }; -} // namespace clang::tidy::modernize +} // namespace clang::tidy::readability -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESPANFIRSTLASTCHECK_H \ No newline at end of file +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_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 04a45d002c0d1d..a52b778a27b5f4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -145,7 +145,7 @@ New checks New check aliases ^^^^^^^^^^^^^^^^^ -- New check `modernize-use-span-first-last` has been added that suggests using +- New check `readability-use-span-first-last` has been added that suggests using ``std::span::first()`` and ``std::span::last()`` member functions instead of equivalent ``subspan()``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst similarity index 92% rename from clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst rename to clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst index 78cd900885ac3f..ff5c0d53feeb15 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst @@ -1,6 +1,6 @@ -.. title:: clang-tidy - modernize-use-starts-ends-with +.. title:: clang-tidy - readability-use-starts-ends-with -modernize-use-starts-ends-with +readability-use-starts-ends-with ============================== Checks for common roundabout ways to express ``starts_with`` and ``ends_with`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp similarity index 95% rename from clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp rename to clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp index cb78bc02f22d4f..93ae95bdd8b6b4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-subspan-conversion.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-span-first-last.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s modernize-use-span-first-last %t +// RUN: %check_clang_tidy %s readability-use-span-first-last %t namespace std { template <typename T> >From 42d1df2f5f6b0527d5066a9fbd8ff2f445d2d79a Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:37:32 +0100 Subject: [PATCH 05/23] format --- clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index b2a8f9dd20adba..18607593320635 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -122,7 +122,6 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); - } }; >From 04f3edc9edb5148652bea8a8b5066f32d321ed2b Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:37:47 +0100 Subject: [PATCH 06/23] format --- .../checks/modernize/use-span-first-last.rst | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst 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 deleted file mode 100644 index e8aad59bb2264f..00000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-span-first-last.rst +++ /dev/null @@ -1,19 +0,0 @@ -.. 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 >From 2f9cc3e878a3c053240a7e6f06b304f89c0d6085 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:43:56 +0100 Subject: [PATCH 07/23] format --- .../clang-tidy/readability/UseSpanFirstLastCheck.cpp | 2 ++ .../clang-tidy/checkers/readability/use-span-first-last.cpp | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index da7d147565fbdf..345a197551d618 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -17,6 +17,8 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus20) + return; // Match span::subspan calls const auto HasSpanType = hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( 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 index 93ae95bdd8b6b4..d99a535e012fa3 100644 --- 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 @@ -1,4 +1,6 @@ -// RUN: %check_clang_tidy %s readability-use-span-first-last %t +// RUN: %check_clang_tidy -std=c++20 %s readability-use-span-first-last %t + + namespace std { template <typename T> >From 24690e3a1155b222436b082c4236c933766a8707 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:53:12 +0100 Subject: [PATCH 08/23] up --- .../readability/UseSpanFirstLastCheck.cpp | 34 ++++++++++++------- .../readability/use-span-first-last.cpp | 7 ++-- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index 345a197551d618..5cd0cd0756e3ba 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -19,6 +19,7 @@ namespace clang::tidy::readability { void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus20) return; + // Match span::subspan calls const auto HasSpanType = hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( @@ -41,7 +42,6 @@ void UseSpanFirstLastCheck::check(const MatchFinder::MatchResult &Result) { void UseSpanFirstLastCheck::handleSubspanCall( const MatchFinder::MatchResult &Result, const CXXMemberCallExpr *Call) { - // Get arguments unsigned NumArgs = Call->getNumArgs(); if (NumArgs == 0 || NumArgs > 2) return; @@ -49,14 +49,28 @@ void UseSpanFirstLastCheck::handleSubspanCall( 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 + // Check if this is subspan(0, n) -> first(n) + bool IsZeroOffset = false; const Expr *OffsetE = Offset->IgnoreImpCasts(); if (const auto *IL = dyn_cast<IntegerLiteral>(OffsetE)) { IsZeroOffset = IL->getValue() == 0; } + // Check if this is subspan(size() - n) -> last(n) + bool IsSizeMinusN = false; + const Expr *SizeMinusArg = nullptr; + if (const auto *BO = dyn_cast<BinaryOperator>(OffsetE)) { + if (BO->getOpcode() == BO_Sub) { + if (const auto *SizeCall = dyn_cast<CXXMemberCallExpr>(BO->getLHS())) { + if (SizeCall->getMethodDecl()->getName() == "size") { + IsSizeMinusN = true; + SizeMinusArg = BO->getRHS(); + } + } + } + } + // Build replacement text std::string Replacement; if (IsZeroOffset && Count) { @@ -70,20 +84,17 @@ void UseSpanFirstLastCheck::handleSubspanCall( 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()), + } else if (IsSizeMinusN && SizeMinusArg) { + // subspan(size() - n) -> last(n) + auto ArgStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(SizeMinusArg->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() + ")"; + Replacement = BaseStr.str() + ".last(" + ArgStr.str() + ")"; } if (!Replacement.empty()) { @@ -96,5 +107,4 @@ void UseSpanFirstLastCheck::handleSubspanCall( } } } - } // namespace clang::tidy::readability \ No newline at end of file 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 index d99a535e012fa3..e775de8b806ad8 100644 --- 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 @@ -1,7 +1,5 @@ // RUN: %check_clang_tidy -std=c++20 %s readability-use-span-first-last %t - - namespace std { template <typename T> class span { @@ -39,9 +37,9 @@ void test() { // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer span::first() over subspan() // CHECK-FIXES: auto sub1 = s.first(3); - auto sub2 = s.subspan(2); + auto sub2 = s.subspan(s.size() - 2); // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer span::last() over subspan() - // CHECK-FIXES: auto sub2 = s.last(s.size() - 2); + // CHECK-FIXES: auto sub2 = s.last(2); __SIZE_TYPE__ n = 2; auto sub3 = s.subspan(0, n); @@ -49,4 +47,5 @@ void test() { // CHECK-FIXES: auto sub3 = s.first(n); auto sub4 = s.subspan(1, 2); // No warning + auto sub5 = s.subspan(2); // No warning } \ No newline at end of file >From 39833d470eea5a0f446b01dba8bbc8271ec69366 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:56:54 +0100 Subject: [PATCH 09/23] up --- .../readability/use-span-first-last.rst | 23 +++++++++++++++++++ .../readability/use-starts-ends-with.rst | 22 ------------------ 2 files changed, 23 insertions(+), 22 deletions(-) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst 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 00000000000000..1872ed56583acb --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst @@ -0,0 +1,23 @@ +.. title:: clang-tidy - readability-use-span-first-last + +readability-use-span-first-last +============================= + +Checks for uses of ``std::span::subspan()`` that can be replaced with clearer +``first()`` or ``last()`` member functions. 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. \ No newline at end of file diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst deleted file mode 100644 index ff5c0d53feeb15..00000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/use-starts-ends-with.rst +++ /dev/null @@ -1,22 +0,0 @@ -.. title:: clang-tidy - readability-use-starts-ends-with - -readability-use-starts-ends-with -============================== - -Checks for common roundabout ways to express ``starts_with`` and ``ends_with`` -and suggests replacing with the simpler method when it is available. Notably, -this will work with ``std::string`` and ``std::string_view``. - -Covered scenarios: - -==================================================== ===================== -Expression Replacement ----------------------------------------------------- --------------------- -``u.find(v) == 0`` ``u.starts_with(v)`` -``u.rfind(v, 0) != 0`` ``!u.starts_with(v)`` -``u.compare(0, v.size(), v) == 0`` ``u.starts_with(v)`` -``u.substr(0, v.size()) == v`` ``u.starts_with(v)`` -``v != u.substr(0, v.size())`` ``!u.starts_with(v)`` -``u.compare(u.size() - v.size(), v.size(), v) == 0`` ``u.ends_with(v)`` -``u.rfind(v) == u.size() - v.size()`` ``u.ends_with(v)`` -==================================================== ===================== >From 964fadc022c33e0e09217b3f8d9eea25b1e7a215 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 10:59:15 +0100 Subject: [PATCH 10/23] format --- .../clang-tidy/readability/UseSpanFirstLastCheck.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h index 271730ed4985a3..354ff6edb83dec 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -13,15 +13,19 @@ namespace clang::tidy::readability { -/// Converts std::span::subspan() calls to the more modern first()/last() -/// methods where applicable. +/// Suggests using clearer std::span member functions first()/last() instead of +/// equivalent subspan() calls 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); +/// 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) >From 79afa9e924afa4cb827085301e44a66db15ba755 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 11:21:06 +0100 Subject: [PATCH 11/23] format --- .../clang-tidy/readability/UseSpanFirstLastCheck.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h index 354ff6edb83dec..84a2d5996a996d 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -13,7 +13,7 @@ namespace clang::tidy::readability { -/// Suggests using clearer std::span member functions first()/last() instead of +/// Suggests using clearer std::span member functions first()/last() instead of /// equivalent subspan() calls where applicable. /// /// For example: >From 31d300fb3b9145f57f731462dc3fa928a255a03a Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 11:22:13 +0100 Subject: [PATCH 12/23] format --- .../docs/clang-tidy/checks/readability/use-span-first-last.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 1872ed56583acb..444a0cc4259fc9 100644 --- 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 @@ -1,7 +1,7 @@ .. title:: clang-tidy - readability-use-span-first-last readability-use-span-first-last -============================= +=============================== Checks for uses of ``std::span::subspan()`` that can be replaced with clearer ``first()`` or ``last()`` member functions. These dedicated methods were added >From c7fd4d5f508d43cd7fdaf7ed9a0006687a4215ed Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Fri, 29 Nov 2024 11:36:54 +0100 Subject: [PATCH 13/23] format --- .../clang-tidy/readability/UseSpanFirstLastCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index 5cd0cd0756e3ba..5e0d8abbada2de 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -107,4 +107,4 @@ void UseSpanFirstLastCheck::handleSubspanCall( } } } -} // namespace clang::tidy::readability \ No newline at end of file +} // namespace clang::tidy::readability >From 4a9fc6e75aa5c76375a99de6ce55bd045d32361e Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Mon, 2 Dec 2024 21:11:07 +0100 Subject: [PATCH 14/23] Update clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp Co-authored-by: EugeneZelenko <eugene.zele...@gmail.com> --- .../clang-tidy/readability/UseSpanFirstLastCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index 5e0d8abbada2de..a1531e4663248e 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -1,4 +1,4 @@ -//===--- UseSpanFirstLastCheck.cpp - clang-tidy-----------------*- C++ -*-===// +//===--- 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. >From 3f604e55cb81c72b36bcdce536be9fe342be79f3 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 00:21:11 +0100 Subject: [PATCH 15/23] Update clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h Co-authored-by: EugeneZelenko <eugene.zele...@gmail.com> --- .../clang-tidy/readability/UseSpanFirstLastCheck.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h index 84a2d5996a996d..0108ac66812bcb 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -1,4 +1,4 @@ -//===--- UseSpanFirstLastCheck.h - clang-tidy-------------------*- C++ -*-===// +//===--- 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. >From 7e2aaa4a84e3dde667a6c0a15918f0fd41bb32cc Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 00:22:06 +0100 Subject: [PATCH 16/23] feed --- .../clang-tidy/readability/CMakeLists.txt | 2 +- .../readability/ReadabilityTidyModule.cpp | 4 +-- .../readability/UseSpanFirstLastCheck.cpp | 33 ++++++++----------- .../readability/UseSpanFirstLastCheck.h | 3 ++ .../readability/use-span-first-last.cpp | 6 ++-- 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index f9f9e8e7f19685..fa243fbf740ccc 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -57,8 +57,8 @@ add_clang_library(clangTidyReadabilityModule STATIC UniqueptrDeleteReleaseCheck.cpp UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp - UseStdMinMaxCheck.cpp UseSpanFirstLastCheck.cpp + UseStdMinMaxCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 9729d080f63a84..6fc163c1dabb5b 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -171,10 +171,10 @@ class ReadabilityModule : public ClangTidyModule { "readability-uppercase-literal-suffix"); CheckFactories.registerCheck<UseAnyOfAllOfCheck>( "readability-use-anyofallof"); - CheckFactories.registerCheck<UseStdMinMaxCheck>( - "readability-use-std-min-max"); 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 index a1531e4663248e..7d7dae92b4a27d 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -17,9 +17,6 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { void UseSpanFirstLastCheck::registerMatchers(MatchFinder *Finder) { - if (!getLangOpts().CPlusPlus20) - return; - // Match span::subspan calls const auto HasSpanType = hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( @@ -53,21 +50,19 @@ void UseSpanFirstLastCheck::handleSubspanCall( // Check if this is subspan(0, n) -> first(n) bool IsZeroOffset = false; const Expr *OffsetE = Offset->IgnoreImpCasts(); - if (const auto *IL = dyn_cast<IntegerLiteral>(OffsetE)) { + if (const auto *IL = dyn_cast<IntegerLiteral>(OffsetE)) IsZeroOffset = IL->getValue() == 0; - } // Check if this is subspan(size() - n) -> last(n) bool IsSizeMinusN = false; const Expr *SizeMinusArg = nullptr; - if (const auto *BO = dyn_cast<BinaryOperator>(OffsetE)) { - if (BO->getOpcode() == BO_Sub) { - if (const auto *SizeCall = dyn_cast<CXXMemberCallExpr>(BO->getLHS())) { - if (SizeCall->getMethodDecl()->getName() == "size") { - IsSizeMinusN = true; - SizeMinusArg = BO->getRHS(); - } - } + + const auto *BO = dyn_cast<BinaryOperator>(OffsetE); + if (BO && BO->getOpcode() == BO_Sub) { + const auto *SizeCall = dyn_cast<CXXMemberCallExpr>(BO->getLHS()); + if (SizeCall && SizeCall->getMethodDecl()->getName() == "size") { + IsSizeMinusN = true; + SizeMinusArg = BO->getRHS(); } } @@ -75,23 +70,23 @@ void UseSpanFirstLastCheck::handleSubspanCall( std::string Replacement; if (IsZeroOffset && Count) { // subspan(0, count) -> first(count) - auto CountStr = Lexer::getSourceText( + const auto CountStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Count->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - auto BaseStr = Lexer::getSourceText( + const StringRef BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); Replacement = BaseStr.str() + ".first(" + CountStr.str() + ")"; } else if (IsSizeMinusN && SizeMinusArg) { // subspan(size() - n) -> last(n) - auto ArgStr = Lexer::getSourceText( + const StringRef ArgStr = Lexer::getSourceText( CharSourceRange::getTokenRange(SizeMinusArg->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - auto BaseStr = Lexer::getSourceText( + const StringRef BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); Replacement = BaseStr.str() + ".last(" + ArgStr.str() + ")"; @@ -99,10 +94,10 @@ void UseSpanFirstLastCheck::handleSubspanCall( if (!Replacement.empty()) { if (IsZeroOffset && Count) { - diag(Call->getBeginLoc(), "prefer span::first() over subspan()") + diag(Call->getBeginLoc(), "prefer 'span::first()' over 'subspan()'") << FixItHint::CreateReplacement(Call->getSourceRange(), Replacement); } else { - diag(Call->getBeginLoc(), "prefer span::last() over subspan()") + diag(Call->getBeginLoc(), "prefer 'span::last()' over 'subspan()'") << FixItHint::CreateReplacement(Call->getSourceRange(), Replacement); } } diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h index 84a2d5996a996d..70d4241bc3852f 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -33,6 +33,9 @@ class UseSpanFirstLastCheck : public ClangTidyCheck { 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; + } private: void handleSubspanCall(const ast_matchers::MatchFinder::MatchResult &Result, 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 index e775de8b806ad8..ce3054930ee322 100644 --- 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 @@ -34,16 +34,16 @@ void test() { std::span<int> s(arr, 5); auto sub1 = s.subspan(0, 3); - // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer span::first() over subspan() + // 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-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-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 >From c70797138807543ed81e3b885e2162238d863384 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 00:35:49 +0100 Subject: [PATCH 17/23] up --- .../checks/modernize/use-starts-ends-with.rst | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst new file mode 100644 index 00000000000000..78cd900885ac3f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst @@ -0,0 +1,22 @@ +.. title:: clang-tidy - modernize-use-starts-ends-with + +modernize-use-starts-ends-with +============================== + +Checks for common roundabout ways to express ``starts_with`` and ``ends_with`` +and suggests replacing with the simpler method when it is available. Notably, +this will work with ``std::string`` and ``std::string_view``. + +Covered scenarios: + +==================================================== ===================== +Expression Replacement +---------------------------------------------------- --------------------- +``u.find(v) == 0`` ``u.starts_with(v)`` +``u.rfind(v, 0) != 0`` ``!u.starts_with(v)`` +``u.compare(0, v.size(), v) == 0`` ``u.starts_with(v)`` +``u.substr(0, v.size()) == v`` ``u.starts_with(v)`` +``v != u.substr(0, v.size())`` ``!u.starts_with(v)`` +``u.compare(u.size() - v.size(), v.size(), v) == 0`` ``u.ends_with(v)`` +``u.rfind(v) == u.size() - v.size()`` ``u.ends_with(v)`` +==================================================== ===================== >From cd2fce318b76bebacfd3ed32009f68a25f055ca7 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 00:37:09 +0100 Subject: [PATCH 18/23] feedback --- .../readability/UseSpanFirstLastCheck.h | 12 +++++----- clang-tools-extra/docs/ReleaseNotes.rst | 10 +++++---- .../readability/use-span-first-last.rst | 13 ++++++----- .../readability/use-span-first-last.cpp | 22 ++++++++++++++++++- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h index 2ff0098e405d8d..843052afb66222 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -13,8 +13,8 @@ namespace clang::tidy::readability { -/// Suggests using clearer std::span member functions first()/last() instead of -/// equivalent subspan() calls where applicable. +/// Suggests using clearer 'std::span' member functions 'first()'/'last()' +/// instead of equivalent 'subspan()' calls where applicable. /// /// For example: /// \code @@ -33,9 +33,9 @@ class UseSpanFirstLastCheck : public ClangTidyCheck { 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; - } + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } private: void handleSubspanCall(const ast_matchers::MatchFinder::MatchResult &Result, @@ -44,4 +44,4 @@ class UseSpanFirstLastCheck : public ClangTidyCheck { } // namespace clang::tidy::readability -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USESPANFIRSTLASTCHECK_H \ No newline at end of file +#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 a52b778a27b5f4..dd73974cf7460c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -142,13 +142,15 @@ New checks Finds cases when an uninstantiated virtual member function in a template class causes cross-compiler incompatibility. -New check aliases -^^^^^^^^^^^^^^^^^ +- New :doc:`readability-use-span-first-last + <clang-tidy/checks/readability/readability-use-span-first-last>` check. -- New check `readability-use-span-first-last` has been added that suggests using - ``std::span::first()`` and ``std::span::last()`` member functions instead of + Suggests using ``std::span::first()`` and ``std::span::last()`` member functions instead of equivalent ``subspan()``. +New check aliases +^^^^^^^^^^^^^^^^^ + - 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/readability/use-span-first-last.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-span-first-last.rst index 444a0cc4259fc9..692403ec7f3d7c 100644 --- 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 @@ -9,12 +9,13 @@ 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)`` -==================================== ================================== +========================== ==================== +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 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 index ce3054930ee322..b4cd9d1f30c14c 100644 --- 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 @@ -48,4 +48,24 @@ void test() { auto sub4 = s.subspan(1, 2); // No warning auto sub5 = s.subspan(2); // No warning -} \ No newline at end of file + + +#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); + +} >From 72610ac900d3c5b8c0a0a6247e7bcf75103a1347 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 00:48:43 +0100 Subject: [PATCH 19/23] feedback --- .../readability/UseSpanFirstLastCheck.cpp | 70 ++++++++++++------- .../readability/use-span-first-last.cpp | 27 +++++++ 2 files changed, 72 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index 7d7dae92b4a27d..6240955db03ec9 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -8,6 +8,7 @@ #include "UseSpanFirstLastCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Lex/Lexer.h" @@ -47,53 +48,71 @@ void UseSpanFirstLastCheck::handleSubspanCall( const Expr *Count = NumArgs > 1 ? Call->getArg(1) : nullptr; auto &Context = *Result.Context; - // Check if this is subspan(0, n) -> first(n) - bool IsZeroOffset = false; - const Expr *OffsetE = Offset->IgnoreImpCasts(); - if (const auto *IL = dyn_cast<IntegerLiteral>(OffsetE)) - IsZeroOffset = IL->getValue() == 0; - - // Check if this is subspan(size() - n) -> last(n) - bool IsSizeMinusN = false; - const Expr *SizeMinusArg = nullptr; - - const auto *BO = dyn_cast<BinaryOperator>(OffsetE); - if (BO && BO->getOpcode() == BO_Sub) { - const auto *SizeCall = dyn_cast<CXXMemberCallExpr>(BO->getLHS()); - if (SizeCall && SizeCall->getMethodDecl()->getName() == "size") { - IsSizeMinusN = true; - SizeMinusArg = BO->getRHS(); + class SubspanVisitor : public RecursiveASTVisitor<SubspanVisitor> { + public: + SubspanVisitor(const ASTContext &Context) : Context(Context) {} + + TraversalKind getTraversalKind() const { + return TK_IgnoreUnlessSpelledInSource; } - } + + bool VisitIntegerLiteral(IntegerLiteral *IL) { + if (IL->getValue() == 0) + IsZeroOffset = true; + return true; + } + + bool VisitBinaryOperator(BinaryOperator *BO) { + if (BO->getOpcode() == BO_Sub) { + if (const auto *SizeCall = dyn_cast<CXXMemberCallExpr>(BO->getLHS())) { + if (SizeCall->getMethodDecl()->getName() == "size") { + IsSizeMinusN = true; + SizeMinusArg = BO->getRHS(); + } + } + } + return true; + } + + bool IsZeroOffset = false; + bool IsSizeMinusN = false; + const Expr *SizeMinusArg = nullptr; + + private: + const ASTContext &Context; + }; + + SubspanVisitor Visitor(Context); + Visitor.TraverseStmt(const_cast<Expr *>(Offset->IgnoreImpCasts())); // Build replacement text std::string Replacement; - if (IsZeroOffset && Count) { + if (Visitor.IsZeroOffset && Count) { // subspan(0, count) -> first(count) - const auto CountStr = Lexer::getSourceText( + auto CountStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Count->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - const StringRef BaseStr = Lexer::getSourceText( + auto BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); Replacement = BaseStr.str() + ".first(" + CountStr.str() + ")"; - } else if (IsSizeMinusN && SizeMinusArg) { + } else if (Visitor.IsSizeMinusN && Visitor.SizeMinusArg) { // subspan(size() - n) -> last(n) - const StringRef ArgStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(SizeMinusArg->getSourceRange()), + auto ArgStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(Visitor.SizeMinusArg->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - const StringRef BaseStr = Lexer::getSourceText( + auto BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); Replacement = BaseStr.str() + ".last(" + ArgStr.str() + ")"; } if (!Replacement.empty()) { - if (IsZeroOffset && Count) { + if (Visitor.IsZeroOffset && Count) { diag(Call->getBeginLoc(), "prefer 'span::first()' over 'subspan()'") << FixItHint::CreateReplacement(Call->getSourceRange(), Replacement); } else { @@ -102,4 +121,5 @@ void UseSpanFirstLastCheck::handleSubspanCall( } } } + } // namespace clang::tidy::readability 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 index b4cd9d1f30c14c..ed3b086837a890 100644 --- 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 @@ -69,3 +69,30 @@ void test() { // 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 +} + +// Test instantiation +void testInt() { + testTemplate<int>(); +} \ No newline at end of file >From b565ef7fbe7c3287bff10a3ec88a386f7c7ab1a7 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 00:50:38 +0100 Subject: [PATCH 20/23] feedback --- .../clang-tidy/readability/UseSpanFirstLastCheck.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index 6240955db03ec9..fcb337661230db 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -89,23 +89,23 @@ void UseSpanFirstLastCheck::handleSubspanCall( std::string Replacement; if (Visitor.IsZeroOffset && Count) { // subspan(0, count) -> first(count) - auto CountStr = Lexer::getSourceText( + const StringRef CountStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Count->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - auto BaseStr = Lexer::getSourceText( + const StringRef BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); Replacement = BaseStr.str() + ".first(" + CountStr.str() + ")"; } else if (Visitor.IsSizeMinusN && Visitor.SizeMinusArg) { // subspan(size() - n) -> last(n) - auto ArgStr = Lexer::getSourceText( + const StringRef ArgStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Visitor.SizeMinusArg->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); const auto *Base = cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - auto BaseStr = Lexer::getSourceText( + const StringRef BaseStr = Lexer::getSourceText( CharSourceRange::getTokenRange(Base->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); Replacement = BaseStr.str() + ".last(" + ArgStr.str() + ")"; >From 3e9b0c652047236bdb0bbaf89ecfdaab431cd0e1 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 20:11:10 +0100 Subject: [PATCH 21/23] feedback --- .../readability/UseSpanFirstLastCheck.cpp | 145 +++++++----------- .../readability/UseSpanFirstLastCheck.h | 4 - .../readability/use-span-first-last.cpp | 4 + 3 files changed, 61 insertions(+), 92 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp index fcb337661230db..6644eac5feadfa 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.cpp @@ -18,108 +18,77 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { 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); + // Match span.subspan(0, n) -> first(n) + Finder->addMatcher( + cxxMemberCallExpr( + callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("subspan"))))), + on(expr(HasSpanType).bind("span_object")), + hasArgument(0, integerLiteral(equals(0))), + hasArgument(1, expr().bind("count")), argumentCountIs(2)) + .bind("first_subspan"), + this); + + // Match span.subspan(size() - n) -> last(n) + const auto SizeCall = cxxMemberCallExpr( + callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("size")))))); + + Finder->addMatcher( + cxxMemberCallExpr( + callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("subspan"))))), + on(expr(HasSpanType).bind("span_object")), + hasArgument(0, binaryOperator(hasOperatorName("-"), hasLHS(SizeCall), + hasRHS(expr().bind("count")))), + argumentCountIs(1)) + .bind("last_subspan"), + this); } void UseSpanFirstLastCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("subspan"); - if (!Call) + const auto *SpanObj = Result.Nodes.getNodeAs<Expr>("span_object"); + if (!SpanObj) return; - handleSubspanCall(Result, Call); -} + StringRef SpanText = Lexer::getSourceText( + CharSourceRange::getTokenRange(SpanObj->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); -void UseSpanFirstLastCheck::handleSubspanCall( - const MatchFinder::MatchResult &Result, const CXXMemberCallExpr *Call) { - unsigned NumArgs = Call->getNumArgs(); - if (NumArgs == 0 || NumArgs > 2) - return; + if (const auto *FirstCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("first_subspan")) { + const auto *Count = Result.Nodes.getNodeAs<Expr>("count"); + if (!Count) + return; - const Expr *Offset = Call->getArg(0); - const Expr *Count = NumArgs > 1 ? Call->getArg(1) : nullptr; - auto &Context = *Result.Context; - - class SubspanVisitor : public RecursiveASTVisitor<SubspanVisitor> { - public: - SubspanVisitor(const ASTContext &Context) : Context(Context) {} - - TraversalKind getTraversalKind() const { - return TK_IgnoreUnlessSpelledInSource; - } - - bool VisitIntegerLiteral(IntegerLiteral *IL) { - if (IL->getValue() == 0) - IsZeroOffset = true; - return true; - } - - bool VisitBinaryOperator(BinaryOperator *BO) { - if (BO->getOpcode() == BO_Sub) { - if (const auto *SizeCall = dyn_cast<CXXMemberCallExpr>(BO->getLHS())) { - if (SizeCall->getMethodDecl()->getName() == "size") { - IsSizeMinusN = true; - SizeMinusArg = BO->getRHS(); - } - } - } - return true; - } - - bool IsZeroOffset = false; - bool IsSizeMinusN = false; - const Expr *SizeMinusArg = nullptr; - - private: - const ASTContext &Context; - }; - - SubspanVisitor Visitor(Context); - Visitor.TraverseStmt(const_cast<Expr *>(Offset->IgnoreImpCasts())); - - // Build replacement text - std::string Replacement; - if (Visitor.IsZeroOffset && Count) { - // subspan(0, count) -> first(count) - const StringRef CountStr = Lexer::getSourceText( + StringRef CountText = Lexer::getSourceText( CharSourceRange::getTokenRange(Count->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); - const auto *Base = - cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - const StringRef BaseStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(Base->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); - Replacement = BaseStr.str() + ".first(" + CountStr.str() + ")"; - } else if (Visitor.IsSizeMinusN && Visitor.SizeMinusArg) { - // subspan(size() - n) -> last(n) - const StringRef ArgStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(Visitor.SizeMinusArg->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); - const auto *Base = - cast<CXXMemberCallExpr>(Call)->getImplicitObjectArgument(); - const StringRef BaseStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(Base->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); - Replacement = BaseStr.str() + ".last(" + ArgStr.str() + ")"; + *Result.SourceManager, Result.Context->getLangOpts()); + + std::string Replacement = + SpanText.str() + ".first(" + CountText.str() + ")"; + + diag(FirstCall->getBeginLoc(), "prefer 'span::first()' over 'subspan()'") + << FixItHint::CreateReplacement(FirstCall->getSourceRange(), + Replacement); } - if (!Replacement.empty()) { - if (Visitor.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); - } + if (const auto *LastCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("last_subspan")) { + const auto *Count = Result.Nodes.getNodeAs<Expr>("count"); + if (!Count) + return; + + StringRef CountText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Count->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + std::string Replacement = SpanText.str() + ".last(" + CountText.str() + ")"; + + diag(LastCall->getBeginLoc(), "prefer 'span::last()' over 'subspan()'") + << FixItHint::CreateReplacement(LastCall->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 index 843052afb66222..69036df9f07061 100644 --- a/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseSpanFirstLastCheck.h @@ -36,10 +36,6 @@ class UseSpanFirstLastCheck : public ClangTidyCheck { bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus20; } - -private: - void handleSubspanCall(const ast_matchers::MatchFinder::MatchResult &Result, - const CXXMemberCallExpr *Call); }; } // namespace clang::tidy::readability 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 index ed3b086837a890..358e6689d66a40 100644 --- 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 @@ -90,6 +90,10 @@ void testTemplate() { 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 >From 2c65d529843eeb843ac0f0f995be179c973f9156 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Tue, 3 Dec 2024 20:15:33 +0100 Subject: [PATCH 22/23] feedback --- .../readability/use-span-first-last.rst | 20 +++++++++---------- .../readability/use-span-first-last.cpp | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) 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 index 692403ec7f3d7c..f1c23ef3920801 100644 --- 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 @@ -3,22 +3,22 @@ readability-use-span-first-last =============================== -Checks for uses of ``std::span::subspan()`` that can be replaced with clearer -``first()`` or ``last()`` member functions. These dedicated methods were added -to C++20 to provide more expressive alternatives to common subspan operations. +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)`` -========================== ==================== +=============================== ============ +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. \ No newline at end of file +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 index 358e6689d66a40..a4cf1467d0b7e4 100644 --- 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 @@ -99,4 +99,4 @@ void testTemplate() { // Test instantiation void testInt() { testTemplate<int>(); -} \ No newline at end of file +} >From 8c16f74f48e312def7198590374338a96e2f83e6 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <hel...@januschka.com> Date: Wed, 4 Dec 2024 21:32:10 +0100 Subject: [PATCH 23/23] up --- .../checks/readability/use-span-first-last.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 index f1c23ef3920801..6aa6cdd81d2fe5 100644 --- 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 @@ -9,12 +9,12 @@ 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)`` -=============================== ============ +=========================== ============ +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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits