[clang-tidy]suggest use std::span as replacement of c array in C++20 for modernize-avoid-c-arrays (#108555)

HerrCai0907 · web-flow · commit 785624b28237 · 2024-09-18T08:03:06.000+08:00
The incompleted C-Array in parameter does not own memory. Instead, It is
equivalent to pointer.
So we should not use `std::array` as recommended modern C++ replacement,
but use `std::vector` and `std::span` in C++20
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
@@ -9,6 +9,7 @@
 #include "AvoidCArraysCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 
 using namespace clang::ast_matchers;
 
@@ -60,6 +61,7 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
 
   Finder->addMatcher(
       typeLoc(hasValidBeginLoc(), hasType(arrayType()),
+              optionally(hasParent(parmVarDecl().bind("param_decl"))),
               unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
                            hasParent(varDecl(isExternC())),
                            hasParent(fieldDecl(
@@ -72,11 +74,28 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
 
 void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *ArrayType = Result.Nodes.getNodeAs<TypeLoc>("typeloc");
-
+  const bool IsInParam =
+      Result.Nodes.getNodeAs<ParmVarDecl>("param_decl") != nullptr;
+  const bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType();
+  enum class RecommendType { Array, Vector, Span };
+  llvm::SmallVector<const char *> RecommendTypes{};
+  if (IsVLA) {
+    RecommendTypes.push_back("std::vector<>");
+  } else if (ArrayType->getTypePtr()->isIncompleteArrayType() && IsInParam) {
+    // in function parameter, we also don't know the size of
+    // IncompleteArrayType.
+    if (Result.Context->getLangOpts().CPlusPlus20)
+      RecommendTypes.push_back("std::span<>");
+    else {
+      RecommendTypes.push_back("std::array<>");
+      RecommendTypes.push_back("std::vector<>");
+    }
+  } else {
+    RecommendTypes.push_back("std::array<>");
+  }
   diag(ArrayType->getBeginLoc(),
-       "do not declare %select{C-style|C VLA}0 arrays, use "
-       "%select{std::array<>|std::vector<>}0 instead")
-      << ArrayType->getTypePtr()->isVariableArrayType();
+       "do not declare %select{C-style|C VLA}0 arrays, use %1 instead")
+      << IsVLA << llvm::join(RecommendTypes, " or ");
 }
 
 } // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -137,6 +137,11 @@ Changes in existing checks
   <clang-tidy/checks/misc/definitions-in-headers>` check by rewording the
   diagnostic note that suggests adding ``inline``.
 
+- Improved :doc:`modernize-avoid-c-arrays
+  <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using ``std::span``
+  as a replacement for parameters of incomplete C array type in C++20 and 
+  ``std::array`` or ``std::vector`` before C++20.
+
 - Improved :doc:`modernize-use-std-format
   <clang-tidy/checks/modernize/use-std-format>` check to support replacing
   member function calls too.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst
@@ -10,6 +10,9 @@ modernize-avoid-c-arrays
 Finds C-style array types and recommend to use ``std::array<>`` /
 ``std::vector<>``. All types of C arrays are diagnosed.
 
+For incomplete C-style array types appeared in parameters, It would be better to
+use ``std::span`` / ``gsl::span`` as replacement.
+
 However, fix-it are potentially dangerous in header files and are therefore not
 emitted right now.
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-c++20.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-c++20.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-avoid-c-arrays %t
+
+int f1(int data[], int size) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: do not declare C-style arrays, use std::span<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+}
+
+int f2(int data[100]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: do not declare C-style arrays, use std::array<> instead
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-main.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-main.cpp
@@ -1,7 +1,7 @@
-// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+// RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t
 
 int not_main(int argc, char *argv[]) {
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead
   int f4[] = {1, 2};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
 }
@@ -11,7 +11,7 @@ int main(int argc, char *argv[]) {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
 
   auto not_main = [](int argc, char *argv[]) {
-    // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style arrays, use std::array<> instead
+    // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead
     int f6[] = {1, 2};
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
   };
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t -- \
+// RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t -- \
 // RUN:  -config='{CheckOptions: { modernize-avoid-c-arrays.AllowStringArrays: true }}'
 
 const char name[] = "name";
 const char array[] = {'n', 'a', 'm', 'e', '\0'};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
 
 void takeCharArray(const char name[]);
-// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead [modernize-avoid-c-arrays]
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-three-arg-main.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-three-arg-main.cpp
@@ -1,8 +1,8 @@
-// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+// RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t
 
 int not_main(int argc, char *argv[], char *argw[]) {
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead
-  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead
   int f4[] = {1, 2};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
 }
@@ -12,8 +12,8 @@ int main(int argc, char *argv[], char *argw[]) {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
 
   auto not_main = [](int argc, char *argv[], char *argw[]) {
-    // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style arrays, use std::array<> instead
-    // CHECK-MESSAGES: :[[@LINE-2]]:46: warning: do not declare C-style arrays, use std::array<> instead
+    // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead
+    // CHECK-MESSAGES: :[[@LINE-2]]:46: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead
     int f6[] = {1, 2};
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
   };
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+// RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t
 
 int a[] = {1, 2};
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
@@ -91,4 +91,4 @@ const char name[] = "Some string";
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
 
 void takeCharArray(const char name[]);
-// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use std::array<> or std::vector<> instead [modernize-avoid-c-arrays]
