diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp index ee70911dff1d5..a5b535f7433bb 100644 --- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp @@ -39,6 +39,53 @@ AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) { return FD ? FD->isMain() : false; } +template +const TargetType *getAs(const NodeType *Node) { + if constexpr (std::is_same_v) + return Node->template get(); + else + return llvm::dyn_cast(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(Node)) || + IsImplicitInstantiation(getAs(Node)) || + IsImplicitInstantiation(getAs(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())) + 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 ArrayTypeLoc{}; + + if (const auto *MatchedTypeLoc = Result.Nodes.getNodeAs("typeloc")) + ArrayTypeLoc = *MatchedTypeLoc; + + if (const auto *TemplateArgArrayTypeLoc = + Result.Nodes.getNodeAs("template_arg_array_typeloc")) + ArrayTypeLoc = *TemplateArgArrayTypeLoc; + + assert(!ArrayTypeLoc.isNull()); + const bool IsInParam = Result.Nodes.getNodeAs("param_decl") != nullptr; - const bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType(); + const bool IsVLA = ArrayTypeLoc.getTypePtr()->isVariableArrayType(); enum class RecommendType { Array, Vector, Span }; llvm::SmallVector 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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 32e4dfb8aa329..e3d4e6d67d9ba 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -199,6 +199,10 @@ Changes in existing checks - Improved :doc:`misc-header-include-cycle ` check performance. +- Improved :doc:`modernize-avoid-c-arrays + ` to not diagnose array types + which are part of an implicit instantiation of a template. + - Improved :doc:`modernize-use-constraints ` check by fixing a crash on uses of non-standard ``enable_if`` with a signature different from 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 index 906663828a547..18458bd47d347 100644 --- 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 @@ -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 +// 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 +// 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() {} 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 index 14eb2852c639a..95e04d53eb6ac 100644 --- 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 @@ -32,7 +32,14 @@ array 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 dk; +// no diagnostic expected here since no concrete C-style array type is written here + +array ek; +// no diagnostic expected here since no concrete C-style array type is written here template 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 + struct is_same { constexpr static bool value{false}; }; + + template + struct is_same { constexpr static bool value{true}; }; + + template + constexpr bool is_same_v = is_same::value; + + template struct remove_const { typedef T type; }; + template struct remove_const { typedef T type; }; + + template + using remove_const_t = typename remove_const::type; + + template struct enable_if {}; + template struct enable_if { typedef T type; }; + + template< bool B, class T = void > + using enable_if_t = typename enable_if::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 , + bool = std::is_same::value, + bool = std::is_same_v, int>, + bool = std::is_same, int>::value, + bool = std::is_same_v::type, int>, + bool = std::is_same::type, int>::value, + std::enable_if_t, int>) && not(std::is_same_v::type, char>), bool> = true, + typename std::enable_if, int>) && not(std::is_same_v::type, char>), bool>::type = true, + typename = std::enable_if_t, int>) && not(std::is_same_v::type, char>)>, + typename = typename std::remove_const::type, + typename = std::remove_const_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 , + // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] + bool = std::is_same::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, int[]>) && not(std::is_same_v::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::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> + // 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; +// 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; +// 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 , + bool = std::is_same::value, + bool = std::is_same_v, int>, + bool = std::is_same, int>::value, + bool = std::is_same_v::type, int>, + bool = std::is_same::type, int>::value, + std::enable_if_t, int>) && not(std::is_same_v::type, char>), bool> = true, + typename std::enable_if, int>) && not(std::is_same_v::type, char>), bool>::type = true, + typename = std::enable_if_t, int>) && not(std::is_same_v::type, char>)>, + typename = typename std::remove_const::type, + typename = std::remove_const_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 , + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] + bool = std::is_same::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, int[]>) && not(std::is_same_v::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::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> + // 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 +T some_constant{}; + +// explicit instantiations +template +int some_constant[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[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[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[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({}); + fun({}); + + func({}); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] + fun({}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] + + MyClassTemplate var{t}; + MyClassTemplate var1{{}}; + MyClassTemplate 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 var4{{}}; + decltype(var1)::MyInnerClassTemplate var5{{}}; + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] + + MyClassTemplate::MyInnerClassTemplate var6{t}; + MyClassTemplate::MyInnerClassTemplate var7{{}}; + MyClassTemplate::MyInnerClassTemplate var8{{}}; + // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] + MyClassTemplate::MyInnerClassTemplate 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] +}