[clang-tidy] do not diagnose array types within implicit instantiations of a template

clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp

@@ -39,6 +39,53 @@ AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
   return FD ? FD->isMain() : false;
 }
 
+template <typename TargetType, typename NodeType>
+const TargetType *getAs(const NodeType *Node) {
+  if constexpr (std::is_same_v<NodeType, clang::DynTypedNode>)
+    return Node->template get<TargetType>();
+  else
+    return llvm::dyn_cast<TargetType>(Node);
+}
+
+AST_MATCHER(clang::TypeLoc, isWithinImplicitTemplateInstantiation) {
+  const auto IsImplicitTemplateInstantiation = [](const auto *Node) {
+    const auto IsImplicitInstantiation = [](const auto *Node) {
+      return (Node != nullptr) && (Node->getTemplateSpecializationKind() ==
+                                   TSK_ImplicitInstantiation);
+    };
+    return (IsImplicitInstantiation(getAs<clang::CXXRecordDecl>(Node)) ||
+            IsImplicitInstantiation(getAs<clang::FunctionDecl>(Node)) ||
+            IsImplicitInstantiation(getAs<clang::VarDecl>(Node)));
+  };
+
+  DynTypedNodeList ParentNodes = Finder->getASTContext().getParents(Node);
+  const clang::NamedDecl *ParentDecl = nullptr;
+  while (!ParentNodes.empty()) {
+    const DynTypedNode &ParentNode = ParentNodes[0];
+    if (IsImplicitTemplateInstantiation(&ParentNode))
+      return true;
+
+    // in case of a `NamedDecl` as parent node, it is more efficient to proceed
+    // with the upward traversal via DeclContexts (see below) instead of via
+    // parent nodes
+    if ((ParentDecl = ParentNode.template get<clang::NamedDecl>()))
+      break;
+
+    ParentNodes = Finder->getASTContext().getParents(ParentNode);
+  }
+
+  if (ParentDecl != nullptr) {
+    const clang::DeclContext *DeclContext = ParentDecl->getDeclContext();
+    while (DeclContext != nullptr) {
+      if (IsImplicitTemplateInstantiation(DeclContext))
+        return true;
+      DeclContext = DeclContext->getParent();
+    }
+  }
+
+  return false;
+}
+
 } // namespace
 
 AvoidCArraysCheck::AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context)
@@ -66,22 +113,38 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
                            hasParent(varDecl(isExternC())),
                            hasParent(fieldDecl(
                                hasParent(recordDecl(isExternCContext())))),
-                           hasAncestor(functionDecl(isExternC())))),
+                           hasAncestor(functionDecl(isExternC())),
+                           isWithinImplicitTemplateInstantiation())),
               IgnoreStringArrayIfNeededMatcher)
           .bind("typeloc"),
       this);
+
+  Finder->addMatcher(
+      templateArgumentLoc(hasTypeLoc(
+          typeLoc(hasType(arrayType())).bind("template_arg_array_typeloc"))),
+      this);
 }
 
 void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *ArrayType = Result.Nodes.getNodeAs<TypeLoc>("typeloc");
+  TypeLoc ArrayTypeLoc{};
+
+  if (const auto *MatchedTypeLoc = Result.Nodes.getNodeAs<TypeLoc>("typeloc"))
+    ArrayTypeLoc = *MatchedTypeLoc;
+
+  if (const auto *TemplateArgArrayTypeLoc =
+          Result.Nodes.getNodeAs<TypeLoc>("template_arg_array_typeloc"))
+    ArrayTypeLoc = *TemplateArgArrayTypeLoc;
+
+  assert(!ArrayTypeLoc.isNull());
+
   const bool IsInParam =
       Result.Nodes.getNodeAs<ParmVarDecl>("param_decl") != nullptr;
-  const bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType();
+  const bool IsVLA = ArrayTypeLoc.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) {
+  } else if (ArrayTypeLoc.getTypePtr()->isIncompleteArrayType() && IsInParam) {
     // in function parameter, we also don't know the size of
     // IncompleteArrayType.
     if (Result.Context->getLangOpts().CPlusPlus20)
@@ -93,9 +156,11 @@ void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) {
   } else {
     RecommendTypes.push_back("'std::array'");
   }
-  diag(ArrayType->getBeginLoc(),
+
+  diag(ArrayTypeLoc.getBeginLoc(),
        "do not declare %select{C-style|C VLA}0 arrays, use %1 instead")
-      << IsVLA << llvm::join(RecommendTypes, " or ");
+      << IsVLA << llvm::join(RecommendTypes, " or ")
+      << ArrayTypeLoc.getSourceRange();
 }
 
 } // namespace clang::tidy::modernize

clang-tools-extra/docs/ReleaseNotes.rst

@@ -199,6 +199,10 @@ Changes in existing checks
 - Improved :doc:`misc-header-include-cycle
   <clang-tidy/checks/misc/header-include-cycle>` check performance.
 
+- Improved :doc:`modernize-avoid-c-arrays
+  <clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types
+  which are part of an implicit instantiation of a template.
+
 - Improved :doc:`modernize-use-constraints
   <clang-tidy/checks/modernize/use-constraints>` check by fixing a crash on
   uses of non-standard ``enable_if`` with a signature different from

clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp

@@ -7,3 +7,15 @@ const char array[] = {'n', 'a', 'm', 'e', '\0'};
 
 void takeCharArray(const char name[]);
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use 'std::array' or 'std::vector' instead [modernize-avoid-c-arrays]
+
+template <typename T = const char[10], typename U = char[10], char[10]>
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-2]]:53: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-3]]:63: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+void func() {}
+
+template <typename T = const char[], typename U = char[], char[]>
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-2]]:51: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-3]]:59: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+void fun() {}

clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp

@@ -32,7 +32,14 @@ array<int[4], 2> d;
 using k = int[4];
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays, use 'std::array' instead
 
+k ak;
+// no diagnostic expected here since no concrete C-style array type is written here
+
 array<k, 2> dk;
+// no diagnostic expected here since no concrete C-style array type is written here
+
+array<decltype(ak), 3> ek;
+// no diagnostic expected here since no concrete C-style array type is written here
 
 template <typename T>
 class unique_ptr {
@@ -92,3 +99,188 @@ const char name[] = "Some string";
 
 void takeCharArray(const char name[]);
 // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use 'std::array' or 'std::vector' instead [modernize-avoid-c-arrays]
+
+namespace std {
+  template<class T, class U>
+  struct is_same { constexpr static bool value{false}; };
+
+  template<class T>
+  struct is_same<T, T> { constexpr static bool value{true}; };
+
+  template<class T, class U>
+  constexpr bool is_same_v = is_same<T, U>::value;
+
+  template<class T> struct remove_const { typedef T type; };
+  template<class T> struct remove_const<const T> { typedef T type; };
+
+  template<class T>
+  using remove_const_t = typename remove_const<T>::type;
+
+  template<bool B, class T = void> struct enable_if {};
+  template<class T> struct enable_if<true, T> { typedef T type; };
+
+  template< bool B, class T = void >
+  using enable_if_t = typename enable_if<B, T>::type;
+}
+
+// within below template decl, no array type findings are expected within the template parameter declarations since not a single C-style array type got written explicitly
+template <typename T,
+          bool = std::is_same_v<T, int>,
+          bool = std::is_same<T, int>::value,
+          bool = std::is_same_v<std::remove_const_t<T>, int>,
+          bool = std::is_same<std::remove_const_t<T>, int>::value,
+          bool = std::is_same_v<typename std::remove_const<T>::type, int>,
+          bool = std::is_same<typename std::remove_const<T>::type, int>::value,
+          std::enable_if_t<not(std::is_same_v<std::remove_const_t<T>, int>) && not(std::is_same_v<typename std::remove_const<T>::type, char>), bool> = true,
+          typename std::enable_if<not(std::is_same_v<std::remove_const_t<T>, int>) && not(std::is_same_v<typename std::remove_const<T>::type, char>), bool>::type = true,
+          typename = std::enable_if_t<not(std::is_same_v<std::remove_const_t<T>, int>) && not(std::is_same_v<typename std::remove_const<T>::type, char>)>,
+          typename = typename std::remove_const<T>::type,
+          typename = std::remove_const_t<T>>
+class MyClassTemplate {
+ public:
+  // here, however, plenty of array type findings are expected for below template parameter declarations since C-style array types are written explicitly
+  template <typename U = T,
+            bool = std::is_same_v<U, int[]>,
+            // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+            bool = std::is_same<U, int[10]>::value,
+            // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+            std::enable_if_t<not(std::is_same_v<std::remove_const_t<U>, int[]>) && not(std::is_same_v<typename std::remove_const<U>::type, char[10]>), bool> = true,
+            // CHECK-MESSAGES: :[[@LINE-1]]:73: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+            // CHECK-MESSAGES: :[[@LINE-2]]:140: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+            typename = typename std::remove_const<int[10]>::type,
+            // CHECK-MESSAGES: :[[@LINE-1]]:51: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+            typename = std::remove_const_t<int[]>>
+            // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+    class MyInnerClassTemplate {
+     public:
+      MyInnerClassTemplate(const U&) {}
+     private:
+      U field[3];
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+    };
+
+    MyClassTemplate(const T&) {}
+
+ private:
+    T field[7];
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+};
+
+// an explicit instantiation
+template
+class MyClassTemplate<int[2]>;
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+using MyArrayType = int[3];
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+// another explicit instantiation
+template
+class MyClassTemplate<MyArrayType>;
+// no diagnostic is expected here since no C-style array type got written here
+
+// within below template decl, no array type findings are expected within the template parameter declarations since not a single C-style array type got written explicitly
+template <typename T,
+          bool = std::is_same_v<T, int>,
+          bool = std::is_same<T, int>::value,
+          bool = std::is_same_v<std::remove_const_t<T>, int>,
+          bool = std::is_same<std::remove_const_t<T>, int>::value,
+          bool = std::is_same_v<typename std::remove_const<T>::type, int>,
+          bool = std::is_same<typename std::remove_const<T>::type, int>::value,
+          std::enable_if_t<not(std::is_same_v<std::remove_const_t<T>, int>) && not(std::is_same_v<typename std::remove_const<T>::type, char>), bool> = true,
+          typename std::enable_if<not(std::is_same_v<std::remove_const_t<T>, int>) && not(std::is_same_v<typename std::remove_const<T>::type, char>), bool>::type = true,
+          typename = std::enable_if_t<not(std::is_same_v<std::remove_const_t<T>, int>) && not(std::is_same_v<typename std::remove_const<T>::type, char>)>,
+          typename = typename std::remove_const<T>::type,
+          typename = std::remove_const_t<T>>
+void func(const T& param) {
+  int array1[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  T array2[2];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  T value;
+}
+
+// here, however, plenty of array type findings are expected for below template parameter declarations since C-style array types are written explicitly
+template <typename T = int[],
+          // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+          bool = std::is_same_v<T, int[]>,
+          // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+          bool = std::is_same<T, int[10]>::value,
+          // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+          std::enable_if_t<not(std::is_same_v<std::remove_const_t<T>, int[]>) && not(std::is_same_v<typename std::remove_const<T>::type, char[10]>), bool> = true,
+          // CHECK-MESSAGES: :[[@LINE-1]]:71: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+          // CHECK-MESSAGES: :[[@LINE-2]]:138: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+          typename = typename std::remove_const<int[10]>::type,
+          // CHECK-MESSAGES: :[[@LINE-1]]:49: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+          typename = std::remove_const_t<int[]>>
+          // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+void fun(const T& param) {
+  int array3[3];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  T array4[4];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  T value;
+}
+
+template<typename T>
+T some_constant{};
+
+// explicit instantiations
+template
+int some_constant<int[5]>[5];
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+template
+int some_constant<decltype(ak)>[4];
+// no diagnostic is expected here since explicit instantiations aren't represented as `TypeLoc` in the AST and we hence cannot match them as such
+
+MyArrayType mk;
+// no diagnostic is expected here since no C-style array type got written here
+
+// explicit specializations
+template<>
+int some_constant<int[7]>[7]{};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+template<>
+int some_constant<decltype(mk)>[3]{};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+void testArrayInTemplateType() {
+  int t[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  func(t);
+  fun(t);
+
+  func<decltype(t)>({});
+  fun<decltype(t)>({});
+
+  func<int[1]>({});
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+  fun<int[1]>({});
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  MyClassTemplate var{t};
+  MyClassTemplate<decltype(t)> var1{{}};
+  MyClassTemplate<int[2]> var2{{}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  decltype(var1)::MyInnerClassTemplate var3{t};
+  decltype(var1)::MyInnerClassTemplate<decltype(t)> var4{{}};
+  decltype(var1)::MyInnerClassTemplate<char[5]> var5{{}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+
+  MyClassTemplate<decltype(t)>::MyInnerClassTemplate var6{t};
+  MyClassTemplate<decltype(t)>::MyInnerClassTemplate<decltype(t)> var7{{}};
+  MyClassTemplate<decltype(t)>::MyInnerClassTemplate<char[8]> var8{{}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+  MyClassTemplate<int[9]>::MyInnerClassTemplate<char[9]> var9{{}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+  // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
+}