-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[clang-tidy] offer option to check sugared types in avoid-c-arrays check #131468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,12 +21,12 @@ AST_MATCHER(clang::TypeLoc, hasValidBeginLoc) { | |
| return Node.getBeginLoc().isValid(); | ||
| } | ||
|
|
||
| AST_MATCHER_P(clang::TypeLoc, hasType, | ||
| clang::ast_matchers::internal::Matcher<clang::Type>, | ||
| InnerMatcher) { | ||
| AST_MATCHER_P(clang::TypeLoc, hasArrayType, bool, CheckSugaredTypes) { | ||
| const clang::Type *TypeNode = Node.getTypePtr(); | ||
| return TypeNode != nullptr && | ||
| InnerMatcher.matches(*TypeNode, Finder, Builder); | ||
| if (CheckSugaredTypes && TypeNode != nullptr) { | ||
| TypeNode = TypeNode->getUnqualifiedDesugaredType(); | ||
| } | ||
| return TypeNode != nullptr && arrayType().matches(*TypeNode, Finder, Builder); | ||
| } | ||
|
|
||
| AST_MATCHER(clang::RecordDecl, isExternCContext) { | ||
|
|
@@ -43,7 +43,8 @@ AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) { | |
|
|
||
| AvoidCArraysCheck::AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context) | ||
| : ClangTidyCheck(Name, Context), | ||
| AllowStringArrays(Options.get("AllowStringArrays", false)) {} | ||
| AllowStringArrays(Options.get("AllowStringArrays", false)), | ||
| CheckSugaredTypes(Options.get("CheckSugaredTypes", false)) {} | ||
|
|
||
| void AvoidCArraysCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { | ||
| Options.store(Opts, "AllowStringArrays", AllowStringArrays); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add CheckSugaredTypes here |
||
|
|
@@ -60,7 +61,7 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { | |
| unless(parmVarDecl()))))); | ||
|
|
||
| Finder->addMatcher( | ||
| typeLoc(hasValidBeginLoc(), hasType(arrayType()), | ||
| typeLoc(hasValidBeginLoc(), hasArrayType(CheckSugaredTypes), | ||
| optionally(hasParent(parmVarDecl().bind("param_decl"))), | ||
| unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())), | ||
| hasParent(varDecl(isExternC())), | ||
|
|
@@ -96,6 +97,26 @@ void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) { | |
| diag(ArrayType->getBeginLoc(), | ||
| "do not declare %select{C-style|C VLA}0 arrays, use %1 instead") | ||
| << IsVLA << llvm::join(RecommendTypes, " or "); | ||
|
|
||
| if (CheckSugaredTypes) { | ||
| PrintingPolicy PrintPolicy{getLangOpts()}; | ||
| PrintPolicy.SuppressTagKeyword = true; | ||
|
|
||
| const auto TypeName = ArrayType->getType().getAsString(PrintPolicy); | ||
| const auto DesugaredTypeName = ArrayType->getType() | ||
| ->getUnqualifiedDesugaredType() | ||
| ->getCanonicalTypeInternal() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thats internal function, do not use it |
||
| .getAsString(PrintPolicy); | ||
| if (TypeName != DesugaredTypeName) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. avoid using string comparison, compare types |
||
| diag(ArrayType->getBeginLoc(), "declared type '%0' resolves to '%1'", | ||
| DiagnosticIDs::Note) | ||
| << TypeName << DesugaredTypeName; | ||
| } else { | ||
| diag(ArrayType->getBeginLoc(), "array type '%0' here", | ||
| DiagnosticIDs::Note) | ||
| << TypeName; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } // namespace clang::tidy::modernize | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,3 +72,9 @@ can be either ``char* argv[]`` or ``char** argv``, but cannot be | |
| .. code:: c++ | ||
|
|
||
| const char name[] = "Some name"; | ||
|
|
||
| .. option:: CheckSugaredTypes | ||
|
|
||
| When set to `true` (default is `false`), type aliases, decltypes as well as | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, previous option |
||
| template parameter types will get resolved and diagnosed as usage of | ||
| C-style array in case their underlying type resolves to one. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| // RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t -- \ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try merge this file with exist one, you can set macro suffix, to distinguish diagnostic
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just followed the approach for all the existing tests where different use cases got put into separate .cpp files. Was this not correct? |
||
| // RUN: -config='{CheckOptions: { modernize-avoid-c-arrays.CheckSugaredTypes: true }}' | ||
|
|
||
| int a[] = {1, 2}; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| int b[1]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| void foo() { | ||
| int c[b[0]]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C VLA arrays, use 'std::vector' instead | ||
|
|
||
| using d = decltype(c); | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not declare C VLA arrays, use 'std::vector' instead | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:13: note: declared type 'decltype(c)' resolves to 'int[b[0]]' | ||
|
|
||
| d e; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C VLA arrays, use 'std::vector' instead | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:3: note: declared type 'd' resolves to 'int[b[0]]' | ||
| } | ||
|
|
||
| template <typename T, int Size> | ||
| class array { | ||
| T d[Size]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| int e[1]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead | ||
| }; | ||
|
|
||
| array<int[4], 2> d; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| using k = int[4]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| array<k, 2> dk; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use 'std::array' instead | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:7: note: declared type 'k' resolves to 'int[4]' | ||
|
|
||
| template <typename T> | ||
| class unique_ptr { | ||
| T *d; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:3: note: array type 'int[]' here | ||
|
|
||
| int e[1]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead | ||
| }; | ||
|
|
||
| unique_ptr<int[]> d2; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| using k2 = int[]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| unique_ptr<k2> dk2; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use 'std::array' instead | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:12: note: declared type 'k2' resolves to 'int[]' | ||
|
|
||
| // Some header | ||
| extern "C" { | ||
|
|
||
| int f[] = {1, 2}; | ||
|
|
||
| int j[1]; | ||
|
|
||
| inline void bar() { | ||
| { | ||
| int j[j[0]]; | ||
| } | ||
| } | ||
|
|
||
| extern "C++" { | ||
| int f3[] = {1, 2}; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| int j3[1]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| struct Foo { | ||
| int f3[3] = {1, 2}; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead | ||
|
|
||
| int j3[1]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use 'std::array' instead | ||
| }; | ||
| } | ||
|
|
||
| struct Bar { | ||
|
|
||
| int f[3] = {1, 2}; | ||
|
|
||
| int j[1]; | ||
| }; | ||
| } | ||
|
|
||
| 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' or 'std::vector' instead [modernize-avoid-c-arrays] | ||
|
|
||
| using MyIntArray = int[10]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] | ||
|
|
||
| using MyIntArray2D = MyIntArray[10]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:22: note: declared type 'MyIntArray[10]' resolves to 'int[10][10]' | ||
|
|
||
| using MyIntArray3D = MyIntArray2D[10]; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:22: note: declared type 'MyIntArray2D[10]' resolves to 'int[10][10][10]' | ||
|
|
||
| MyIntArray3D array3D; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:1: note: declared type 'MyIntArray3D' resolves to 'int[10][10][10]' | ||
|
|
||
| MyIntArray2D array2D; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:1: note: declared type 'MyIntArray2D' resolves to 'int[10][10]' | ||
|
|
||
| MyIntArray array1D; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays] | ||
| // CHECK-MESSAGES: :[[@LINE-2]]:1: note: declared type 'MyIntArray' resolves to 'int[10]' | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just check if TypeNode is arrayType directly instead of using matchers