feedback

Author: hjanuschka

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

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>();
+}