ccotter updated this revision to Diff 508451. ccotter added a comment. rebase
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145138/new/ https://reviews.llvm.org/D145138 Files: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h clang-tools-extra/clang-tidy/utils/CMakeLists.txt clang-tools-extra/clang-tidy/utils/LexerUtils.cpp clang-tools-extra/clang-tidy/utils/TypeUtils.cpp clang-tools-extra/clang-tidy/utils/TypeUtils.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-cxx17.cpp clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp +++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp @@ -1,4 +1,6 @@ -// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t +// RUN: %check_clang_tidy -std=c++11 %s modernize-avoid-c-arrays %t + +//CHECK-FIXES: #include <array> int a[] = {1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead @@ -86,3 +88,262 @@ int j[1]; }; } + +template <class T> struct TStruct {}; + +void replacements() { + int ar[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 10> ar; + TStruct<int> ar2[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<TStruct<int>, 10> ar2; + TStruct< int > ar3[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<TStruct< int >, 10> ar3; + int * ar4[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int *, 10> ar4; + int * /*comment*/ar5[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int *, 10> /*comment*/ar5; + volatile const int * ar6[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<volatile const int *, 10> ar6; + volatile int ar7[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array<int, 10> ar7; + int const * ar8[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int const *, 10> ar8; + int ar9[1]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 1> ar9; + static int volatile constexpr ar10[10] = {}; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: static volatile constexpr std::array<int, 10> ar10 = { {} }; + thread_local int ar11[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: thread_local std::array<int, 10> ar11; + thread_local/*a*/int/*b*/ar12[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: thread_local/*a*/std::array<int, 10>/*b*/ar12; + /*a*/ int/*b*/ /*c*/*/*d*/ /*e*/ /*f*/ar13[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: /*a*/ std::array<int/*b*/ /*c*/*, 10>/*d*/ /*e*/ /*f*/ar13; + TStruct<int*> ar14[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<TStruct<int*>, 10> ar14; + volatile TStruct<const int*> ar15[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array<TStruct<const int*>, 10> ar15; + TStruct<int const*> ar16[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<TStruct<int const*>, 10> ar16; + TStruct<unsigned const int> ar17[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<TStruct<unsigned const int>, 10> ar17; + volatile int static thread_local * ar18[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: static thread_local std::array<volatile int *, 10> ar18; + + // Note, there is a tab '\t' before the semicolon in the declaration below. + int ar19[3] ; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> ar19 ; + + int + ar20[3]; + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> + // CHECK-FIXES-NEXT: ar20; + + int init[] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> init = { {1,2,3} }; + int init2[3] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> init2 = { {1,2,3} }; + int init3[3] = {1,2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> init3 = { {1,2} }; + char init4[] = "abcdef"; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 7> init4 = { "abcdef" }; + wchar_t init5[] = L"abcdef"; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<wchar_t, 7> init5 = { L"abcdef" }; + char init6[] = ""; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 1> init6 = { "" }; + char init7[] = "\0"; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 2> init7 = { "\0" }; + char init8[] = {"abc"}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 4> init8 = { {"abc"} }; + char init9[] = R"LONG(a + really + long + string)LONG"; + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 27> init9 = { R"LONG(a + // CHECK-FIXES-NEXT: really + // CHECK-FIXES-NEXT: long + // CHECK-FIXES-NEXT: string)LONG" }; + char init10[] = {"abc" "def"}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 7> init10 = { {"abc" "def"} }; + const char* init11 = {nullptr}; + const char* const init12[] = {"abc","def"}; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array<const char*, 2> init12 = { {"abc","def"} }; + const char*const init13[] = {"abc","def"}; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array<const char*, 2>init13 = { {"abc","def"} }; + + // CHECK-MESSAGES: :[[@LINE+2]]:9: warning: do not declare C-style arrays, use std::array<> instead +#define NAME "ABC" + const char init14[] = NAME +#if 1 + " " +#endif + ; + + // Note: there are two tab '\t' characters between the 'int', '*', and 'init15' tokens below. + int * init15[1] = {nullptr}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int *, 1> init15 = { {nullptr} }; + + int two_d[10][5]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: int two_d[10][5]; + int three_d[10][5][3]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: int three_d[10][5][3]; +} + +void replace_cv_within_type() { + // FIXME: clang's AST TypeLoc etc do not give SourceLocations of individual + // qualifiers etc. In combined types (e.g. 'unsigned int') with a cv + // qualifier in the middle of the type (e.g., 'unsigned volatile int'), we + // do not support pulling the qualifier out to appear before the std::array + // decl. + + unsigned volatile int ar1[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: unsigned volatile int ar1[10]; + volatile unsigned int ar2[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array<unsigned int, 10> ar2; + unsigned int volatile ar3[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array<unsigned int, 10> ar3; + unsigned const int* volatile ar4[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array<unsigned const int*, 10> ar4; +} + +void consumes_ptr(int*); +void replacement_with_refs() { + int ar[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 10> ar; + + for (int i = 0; i != sizeof(ar)/sizeof(ar[0]); ++i) { ar[i]++; } + // CHECK-FIXES: for (int i = 0; i != sizeof(ar)/sizeof(ar[0]); ++i) { ar[i]++; } + for (int i: ar) {} + // CHECK-FIXES: for (int i: ar) {} + + int* ptr = ar; + // CHECK-FIXES: int* ptr = ar.begin(); + const int* cptr = ar; + // CHECK-FIXES: const int* cptr = ar.begin(); + ptr = (ar+1); + // CHECK-FIXES: ptr = (ar.begin()+1); + + int a; + a = ar[0] + (ar)[1] + ((ar))[2]; + // CHECK-FIXES: a = ar[0] + (ar)[1] + ((ar))[2]; + a = 0[ar] + (0)[ar] + 0[(ar)]; + // CHECK-FIXES: a = ar[0] + ar[(0)] + (ar)[0]; + a = *(ar+2); + // CHECK-FIXES: a = *(ar.begin()+2); + consumes_ptr(ar); + // CHECK-FIXES: consumes_ptr(ar.begin()); + + unsigned sz = sizeof(ar) + sizeof((ar)) + sizeof(ar[0]) + sizeof ar; + // CHECK-FIXES: unsigned sz = sizeof(ar) + sizeof((ar)) + sizeof(ar[0]) + sizeof ar; +} + +void cannot_replace_reference_taken() { + int ar[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + int (&ref)[10] = ar; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead +} + +struct SomeClass { + void fn1(int); + void fn2(int); +}; +void fn1(int); +void fn2(int); +void cannot_replace_array_of_function_pointers() { + void (*fns1[])(int) = {fn1, fn2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + + using FnPtr = void(*)(int); + FnPtr fns2[] = {fn1, fn2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + + void (SomeClass::*fns3[])(int) = {&SomeClass::fn1, &SomeClass::fn2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + + using MemFnPtr = void(SomeClass::*)(int); + MemFnPtr fns4[] = {&SomeClass::fn1, &SomeClass::fn2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead +} + +void consume_array_ref(int (&)[10]); +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead +void cannot_replace_reference_taken_in_call() { + int ar[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + consume_array_ref(ar); +} + +void vla_not_replaced(int n) { + int ar[n]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C VLA arrays, use std::vector<> instead +} + +struct Obj1 {}; +struct Obj2 { + Obj2(const Obj1& = {}); +}; +void init_expr_with_temp() { + Obj2 ar[] = { {} }; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<Obj2, 1> ar = { { {} } }; +} + +struct AStruct { + #define DEFINES_METHOD_WITH_ARRAY void method() { int ar[10]; } + DEFINES_METHOD_WITH_ARRAY + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead +}; + +void macros_not_replaced() { + int d[3]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead +#define EXPANDS_X(x) x + EXPANDS_X(consumes_ptr(d)); +} + +void attrs_not_replaced() { + [[maybe_unused]] int ar1[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not declare C-style arrays, use std::array<> instead + __attribute__((__unused__)) int ar2[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not declare C-style arrays, use std::array<> instead +} Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-cxx17.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-cxx17.cpp @@ -0,0 +1,131 @@ +// RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t + +//CHECK-FIXES: #include <array> + +template <class T1, class T2> +struct Pair { + T1 t1; + T2 t2; +}; + +struct Obj1 {}; +struct Obj2 { + Obj2(const Obj1& = {}); +}; + +struct StringRef { + StringRef(const char*); + StringRef(const StringRef&); +}; + +void ctad_replacements() { + const int ci1{}, ci2{}; + int i1, i2; + + int ar[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 10> ar; + + int init[] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init = {1,2,3}; + int init2[3] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> init2 = { {1,2,3} }; + char init3[] = "abcdef"; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 7> init3 = { "abcdef" }; + char init4[] = ""; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 1> init4 = { "" }; + char init5[] = {"abc"}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 4> init5 = { {"abc"} }; + char init6[] = {"abc" "def"}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<char, 7> init6 = { {"abc" "def"} }; + int init7[] = {'c',2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<int, 3> init7 = { {'c',2,3} }; + const int init8[] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array init8 = {1,2,3}; + int const init9[] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array init9 = {1,2,3}; + int const init10[] = {i1, i2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array init10 = {i1, i2}; + int const init11[] = {ci1, ci2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array init11 = {ci1, ci2}; + int init12[] = {ci1, ci2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init12 = {ci1, ci2}; + constexpr volatile int static const init13[] = {1,2,3}; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: constexpr volatile static const std::array init13 = {1,2,3}; + const Obj1 init14[] = {Obj1(), Obj1()}; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array init14 = {Obj1(), Obj1()}; + + using IntPair = Pair<int,int>; + IntPair init15[] = {IntPair{1,2}, IntPair{3,4}}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init15 = {IntPair{1,2}, IntPair{3,4}}; + using IntPair = Pair<int,int>; + IntPair init16[] = { {1,2}, {3,4} }; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<IntPair, 2> init16 = { { {1,2}, {3,4} } }; + + StringRef init17[] = {"a", "b"}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<StringRef, 2> init17 = { {"a", "b"} }; + StringRef sr1(""),sr2(""); + StringRef init18[] = {sr1, sr2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init18 = {sr1, sr2}; + StringRef init19[] = {StringRef(""), StringRef("")}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init19 = {StringRef(""), StringRef("")}; + StringRef init20[] = { {""} }; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array<StringRef, 1> init20 = { { {""} } }; + StringRef const init21[] = { StringRef{""} }; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: const std::array init21 = { StringRef{""} }; + + int x0,x1; + int* init22[] = {&x0, &x1}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init22 = {&x0, &x1}; + int*init23[] = {&x0, &x1}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: std::array init23 = {&x0, &x1}; + static thread_local int* const init24[] = {&x0, &x1}; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: static thread_local const std::array init24 = {&x0, &x1}; +} + +void replace_cv_within_type() { + // FIXME: clang's AST TypeLoc etc do not give SourceLocations of individual + // qualifiers etc. In combined types (e.g. 'unsigned int') with a cv + // qualifier in the middle of the type (e.g., 'unsigned volatile int'), we + // do not support pulling the qualifier out to appear before the std::array + // decl. + + unsigned const int ui1{}, ui2{}; + + unsigned volatile int ar1[] = {1u,2u}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: unsigned volatile int ar1[] = {1u,2u}; + volatile unsigned int ar2[] = {1u,2u}; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array ar2 = {1u,2u}; + unsigned int volatile ar3[] = {1u,2u}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array ar3 = {1u,2u}; + unsigned const int* volatile ar4[] = {&ui1, &ui2}; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead + // CHECK-FIXES: volatile std::array ar4 = {&ui1, &ui2}; +} Index: clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst +++ clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst @@ -10,8 +10,8 @@ Finds C-style array types and recommend to use ``std::array<>`` / ``std::vector<>``. All types of C arrays are diagnosed. -However, fix-it are potentially dangerous in header files and are therefore not -emitted right now. +Fix-its are generated for C-style arrays in function bodies. Fix-its are not +provided in other contexts (e.g., class member variables). .. code:: c++ @@ -34,6 +34,17 @@ using k = int[4]; // warning: do not declare C-style arrays, use std::array<> instead + void somefunction() { + int a[10]; // warning: do not declare C-style arrays, use std::array<> instead + // replaced with 'std::array<int, 10> a' + int v = a[0]; + int* ptr = a; // replaced with 'int* ptr = a.begin()' + + int a2[] = {1,2,3}; // warning: do not declare C-style arrays, use std::array<> instead + // In C++14 and older, replaced with 'std::array<int, 3> a2 = {{1,2,3}}' + // In C++17 and newer, replaced with 'std::array a2 = {1,2,3}' + } + However, the ``extern "C"`` code is ignored, since it is common to share such headers between C code, and C++ code. @@ -58,3 +69,11 @@ Similarly, the ``main()`` function is ignored. Its second and third parameters can be either ``char* argv[]`` or ``char** argv``, but cannot be ``std::array<>``. + +Options +------- + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or `google`. Default + is `llvm`. Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -198,6 +198,10 @@ <clang-tidy/checks/misc/unused-using-decls>` check. Global options of the same name should be used instead. +- Improved :doc: `modernize-avoid-c-arrays + <clang-tidy/checks/modernize/avoid-c-arrays>` check to provide fix-its for + C-style arrays declared in function bodies. + - In :doc:`modernize-use-default-member-init <clang-tidy/checks/modernize/use-default-member-init>` count template constructors toward hand written constructors so that they are skipped if more Index: clang-tools-extra/clang-tidy/utils/TypeUtils.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/TypeUtils.h @@ -0,0 +1,42 @@ +//===--- TypeUtils.h - clang-tidy--------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPE_UTILS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPE_UTILS_H + +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Token.h" +#include "llvm/ADT/SmallVector.h" +#include <optional> + +namespace clang { + +class ASTContext; +class Decl; +class Preprocessor; + +namespace tidy::utils::type { + +struct ClassifiedToken { + Token T; + bool IsQualifier; + bool IsSpecifier; +}; + +// Classify the qualifiers and specifier tokens of a declaration. +std::optional<SmallVector<ClassifiedToken, 8>> +classifyDeclTypeTokens(Preprocessor &PP, SourceLocation BeginLoc, + SourceLocation EndLoc, const ASTContext &Ctx); + +std::optional<SmallVector<ClassifiedToken, 8>> +classifyDeclTypeTokens(Preprocessor &PP, const Decl &D, const ASTContext &Ctx); + +} // namespace tidy::utils::type +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPE_UTILS_H Index: clang-tools-extra/clang-tidy/utils/TypeUtils.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/TypeUtils.cpp @@ -0,0 +1,124 @@ +//===--- TypeUtils.cpp - clang-tidy----------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "TypeUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" +#include <optional> + +namespace clang::tidy::utils::type { + +static bool isCvr(Token T) { + return T.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict); +} + +static bool isSpecifier(Token T) { + return T.isOneOf(tok::kw_constexpr, tok::kw_inline, tok::kw_extern, + tok::kw_static, tok::kw_friend, tok::kw_virtual, + tok::kw_thread_local, tok::kw_register); +} + +static std::optional<ClassifiedToken> classifyToken(Preprocessor &PP, + Token Tok) { + ClassifiedToken CT; + CT.T = Tok; + CT.IsQualifier = true; + CT.IsSpecifier = true; + bool ContainsQualifiers = false; + bool ContainsSpecifiers = false; + bool ContainsSomethingElse = false; + + Token End; + End.startToken(); + End.setKind(tok::eof); + SmallVector<Token, 2> Stream{Tok, End}; + + // FIXME: do not report these token to Preprocessor.TokenWatcher. + PP.EnterTokenStream(Stream, false, /*IsReinject=*/false); + while (true) { + Token T; + PP.Lex(T); + if (T.is(tok::eof)) + break; + + const bool Qual = isCvr(T); + const bool Spec = isSpecifier(T); + CT.IsQualifier &= Qual; + CT.IsSpecifier &= Spec; + ContainsQualifiers |= Qual; + ContainsSpecifiers |= Spec; + ContainsSomethingElse |= !Qual && !Spec; + } + + // If the Token/Macro contains more than one type of tokens, we would need + // to split the macro in order to move parts to the trailing return type. + if (ContainsQualifiers + ContainsSpecifiers + ContainsSomethingElse > 1) + return std::nullopt; + + return CT; +} + +static SourceLocation expandIfMacroId(SourceLocation Loc, + const SourceManager &SM) { + if (Loc.isMacroID()) + Loc = expandIfMacroId(SM.getImmediateExpansionRange(Loc).getBegin(), SM); + assert(!Loc.isMacroID() && + "SourceLocation must not be a macro ID after recursive expansion"); + return Loc; +} + +std::optional<SmallVector<ClassifiedToken, 8>> +classifyDeclTypeTokens(Preprocessor &PP, SourceLocation BeginLoc, + SourceLocation EndLoc, const ASTContext &Ctx) { + const SourceManager &SM = Ctx.getSourceManager(); + const LangOptions &LangOpts = Ctx.getLangOpts(); + BeginLoc = expandIfMacroId(BeginLoc, SM); + EndLoc = expandIfMacroId(EndLoc, SM); + + // Create tokens for everything before the name of the function. + std::pair<FileID, unsigned> Loc = SM.getDecomposedLoc(BeginLoc); + StringRef File = SM.getBufferData(Loc.first); + const char *TokenBegin = File.data() + Loc.second; + Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(), + TokenBegin, File.end()); + Token T; + SmallVector<ClassifiedToken, 8> ClassifiedTokens; + while (!Lexer.LexFromRawLexer(T) && + SM.isBeforeInTranslationUnit(T.getLocation(), EndLoc)) { + if (T.is(tok::raw_identifier)) { + IdentifierInfo &Info = Ctx.Idents.get( + StringRef(SM.getCharacterData(T.getLocation()), T.getLength())); + + if (Info.hasMacroDefinition()) { + const MacroInfo *MI = PP.getMacroInfo(&Info); + if (!MI || MI->isFunctionLike()) { + // Cannot handle function style macros. + return std::nullopt; + } + } + + T.setIdentifierInfo(&Info); + T.setKind(Info.getTokenID()); + } + + if (std::optional<ClassifiedToken> CT = classifyToken(PP, T)) + ClassifiedTokens.push_back(*CT); + else + return std::nullopt; + } + + return ClassifiedTokens; +} + +std::optional<SmallVector<ClassifiedToken, 8>> +classifyDeclTypeTokens(Preprocessor &PP, const Decl &D, const ASTContext &Ctx) { + return classifyDeclTypeTokens(PP, D.getBeginLoc(), D.getLocation(), Ctx); +} + +} // namespace clang::tidy::utils::type Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp =================================================================== --- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp +++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp @@ -91,7 +91,7 @@ assert(Range.isValid() && "Invalid Range for relexing provided"); SourceLocation Loc = Range.getBegin(); - while (Loc < Range.getEnd()) { + while (SM.isBeforeInTranslationUnit(Loc, Range.getEnd())) { if (Loc.isMacroID()) return true; Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -20,6 +20,7 @@ RenamerClangTidyCheck.cpp TransformerClangTidyCheck.cpp TypeTraits.cpp + TypeUtils.cpp UsingInserter.cpp LINK_LIBS Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h =================================================================== --- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h +++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h @@ -15,12 +15,6 @@ namespace clang::tidy::modernize { -struct ClassifiedToken { - Token T; - bool IsQualifier; - bool IsSpecifier; -}; - /// Rewrites function signatures to use a trailing return type. /// /// For the user-facing documentation see: @@ -43,10 +37,6 @@ SourceLocation findTrailingReturnTypeSourceLocation( const FunctionDecl &F, const FunctionTypeLoc &FTL, const ASTContext &Ctx, const SourceManager &SM, const LangOptions &LangOpts); - std::optional<SmallVector<ClassifiedToken, 8>> - classifyTokensBeforeFunctionName(const FunctionDecl &F, const ASTContext &Ctx, - const SourceManager &SM, - const LangOptions &LangOpts); SourceRange findReturnTypeAndCVSourceRange(const FunctionDecl &F, const TypeLoc &ReturnLoc, const ASTContext &Ctx, Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp +++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "UseTrailingReturnTypeCheck.h" +#include "../utils/TypeUtils.h" #include "clang/AST/ASTContext.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -20,6 +21,9 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { + +using utils::type::ClassifiedToken; + namespace { struct UnqualNameVisitor : public RecursiveASTVisitor<UnqualNameVisitor> { public: @@ -167,100 +171,6 @@ return Result; } -static bool isCvr(Token T) { - return T.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict); -} - -static bool isSpecifier(Token T) { - return T.isOneOf(tok::kw_constexpr, tok::kw_inline, tok::kw_extern, - tok::kw_static, tok::kw_friend, tok::kw_virtual); -} - -static std::optional<ClassifiedToken> -classifyToken(const FunctionDecl &F, Preprocessor &PP, Token Tok) { - ClassifiedToken CT; - CT.T = Tok; - CT.IsQualifier = true; - CT.IsSpecifier = true; - bool ContainsQualifiers = false; - bool ContainsSpecifiers = false; - bool ContainsSomethingElse = false; - - Token End; - End.startToken(); - End.setKind(tok::eof); - SmallVector<Token, 2> Stream{Tok, End}; - - // FIXME: do not report these token to Preprocessor.TokenWatcher. - PP.EnterTokenStream(Stream, false, /*IsReinject=*/false); - while (true) { - Token T; - PP.Lex(T); - if (T.is(tok::eof)) - break; - - bool Qual = isCvr(T); - bool Spec = isSpecifier(T); - CT.IsQualifier &= Qual; - CT.IsSpecifier &= Spec; - ContainsQualifiers |= Qual; - ContainsSpecifiers |= Spec; - ContainsSomethingElse |= !Qual && !Spec; - } - - // If the Token/Macro contains more than one type of tokens, we would need - // to split the macro in order to move parts to the trailing return type. - if (ContainsQualifiers + ContainsSpecifiers + ContainsSomethingElse > 1) - return std::nullopt; - - return CT; -} - -std::optional<SmallVector<ClassifiedToken, 8>> -UseTrailingReturnTypeCheck::classifyTokensBeforeFunctionName( - const FunctionDecl &F, const ASTContext &Ctx, const SourceManager &SM, - const LangOptions &LangOpts) { - SourceLocation BeginF = expandIfMacroId(F.getBeginLoc(), SM); - SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM); - - // Create tokens for everything before the name of the function. - std::pair<FileID, unsigned> Loc = SM.getDecomposedLoc(BeginF); - StringRef File = SM.getBufferData(Loc.first); - const char *TokenBegin = File.data() + Loc.second; - Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(), - TokenBegin, File.end()); - Token T; - SmallVector<ClassifiedToken, 8> ClassifiedTokens; - while (!Lexer.LexFromRawLexer(T) && - SM.isBeforeInTranslationUnit(T.getLocation(), BeginNameF)) { - if (T.is(tok::raw_identifier)) { - IdentifierInfo &Info = Ctx.Idents.get( - StringRef(SM.getCharacterData(T.getLocation()), T.getLength())); - - if (Info.hasMacroDefinition()) { - const MacroInfo *MI = PP->getMacroInfo(&Info); - if (!MI || MI->isFunctionLike()) { - // Cannot handle function style macros. - diag(F.getLocation(), Message); - return std::nullopt; - } - } - - T.setIdentifierInfo(&Info); - T.setKind(Info.getTokenID()); - } - - if (std::optional<ClassifiedToken> CT = classifyToken(F, *PP, T)) - ClassifiedTokens.push_back(*CT); - else { - diag(F.getLocation(), Message); - return std::nullopt; - } - } - - return ClassifiedTokens; -} - static bool hasAnyNestedLocalQualifiers(QualType Type) { bool Result = Type.hasLocalQualifiers(); if (Type->isPointerType()) @@ -293,9 +203,11 @@ // Include qualifiers to the left and right of the return type. std::optional<SmallVector<ClassifiedToken, 8>> MaybeTokens = - classifyTokensBeforeFunctionName(F, Ctx, SM, LangOpts); - if (!MaybeTokens) + utils::type::classifyDeclTypeTokens(*PP, F, Ctx); + if (!MaybeTokens) { + diag(F.getLocation(), Message); return {}; + } const SmallVector<ClassifiedToken, 8> &Tokens = *MaybeTokens; ReturnTypeRange.setBegin(expandIfMacroId(ReturnTypeRange.getBegin(), SM)); @@ -345,9 +257,11 @@ // Tokenize return type. If it contains macros which contain a mix of // qualifiers, specifiers and types, give up. std::optional<SmallVector<ClassifiedToken, 8>> MaybeTokens = - classifyTokensBeforeFunctionName(F, Ctx, SM, LangOpts); - if (!MaybeTokens) + utils::type::classifyDeclTypeTokens(*PP, F, Ctx); + if (!MaybeTokens) { + diag(F.getLocation(), Message); return; + } // Find specifiers, remove them from the return type, add them to 'auto'. unsigned int ReturnTypeBeginOffset = Index: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h =================================================================== --- clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h +++ clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDCARRAYSCHECK_H #include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" namespace clang::tidy::modernize { @@ -19,13 +20,27 @@ /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-c-arrays.html class AvoidCArraysCheck : public ClangTidyCheck { public: - AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context); + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus11; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + Preprocessor *PP = nullptr; + utils::IncludeInserter IncludeInserter; + bool replaceDecl(ArrayTypeLoc ATL, const VarDecl *Var, bool UseCTAD, + ASTContext &Context, std::vector<FixItHint> &FixIts); + bool replaceArrayReferences(const VarDecl *Var, const FunctionDecl *Func, + ASTContext &Context, + std::vector<FixItHint> &FixIts); + std::vector<FixItHint> replaceArray(ArrayTypeLoc ATL, + const DeclStmt *VarDeclStmt, + const VarDecl *Var, ASTContext &Context); }; } // namespace clang::tidy::modernize Index: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp +++ clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp @@ -7,8 +7,14 @@ //===----------------------------------------------------------------------===// #include "AvoidCArraysCheck.h" +#include "../utils/LexerUtils.h" +#include "../utils/TypeUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" +#include <optional> +#include <tuple> using namespace clang::ast_matchers; @@ -40,9 +46,14 @@ namespace clang::tidy::modernize { +using utils::type::ClassifiedToken; + void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( typeLoc(hasValidBeginLoc(), hasType(arrayType()), + optionally(hasParent(varDecl(hasParent(declStmt().bind("decl")), + hasAncestor(functionDecl())) + .bind("var"))), unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())), hasParent(varDecl(isExternC())), hasParent(fieldDecl( @@ -52,13 +63,523 @@ this); } +SourceLocation findEndOfToken(SourceLocation Loc, const ASTContext &Context) { + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + + SourceLocation Orig = Loc; + Loc = Loc.getLocWithOffset(1); + while (Loc.isValid()) { + SourceLocation PrevTokenLoc = + utils::lexer::findPreviousTokenStart(Loc, SM, LangOpts); + if (PrevTokenLoc != Orig) + return Loc.getLocWithOffset(-1); + Loc = Loc.getLocWithOffset(1); + } + return SourceLocation{}; +} + +SourceLocation getPreviousTokenLoc(SourceLocation Location, + const SourceManager &SM, + const LangOptions &LangOpts, + bool SkipComments = false) { + Token Token; + Token.setKind(tok::unknown); + + Location = Location.getLocWithOffset(-1); + if (Location.isInvalid()) + return Location; + + SourceLocation StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location)); + while (Location != StartOfFile) { + Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts); + if (!Lexer::getRawToken(Location, Token, SM, LangOpts) && + (!SkipComments || !Token.is(tok::comment))) { + break; + } + Location = Location.getLocWithOffset(-1); + } + return Location; +} + +SourceLocation getVarLocForReplacement(const VarDecl *Var, + const ASTContext &Context) { + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + SourceLocation EndLoc = getPreviousTokenLoc(Var->getLocation(), SM, LangOpts); + if (!EndLoc.isValid()) + return SourceLocation{}; + EndLoc = findEndOfToken(EndLoc, Context); + if (!EndLoc.isValid()) + return SourceLocation{}; + CharSourceRange Range{SourceRange{EndLoc, EndLoc.getLocWithOffset(1)}, false}; + StringRef Text = Lexer::getSourceText(Range, SM, LangOpts); + if (Text.empty()) + return SourceLocation{}; + if (std::isspace(Text[0])) + return EndLoc.getLocWithOffset(1); + return EndLoc; +} + +// Collects the qualifiers, specifiers, and type tokens and combines them +// into source text strings suitable for std::array<>/std::vector<> +// replacement. Specifiers are placed outside of the array/vector template +// type, and the qualifiers and type tokens themselves are placed within +// the array/vector template type. +// +// Any text between the final non-specifer token and the following specifier +// token is moved to appear after the ">" in the eventual +// std::array<>/std::vector<> declaration. +// +// The logic works in two passes: first, classifying the tokens and collecting +// the source text associated with each token including any whitespace/comments +// that should remain "attached" to the token. Second, the tokens and text +// are parsed using a "simple" algorithm to determine whether qualifiers are +// associated with the array, or the array element. In particular, handling +// pointers requires extra care to distinguish a qualifier on the pointer type +// vs qualifier on the type being pointed to. Since QualType/TypeLoc do not +// store per qualifier source location information, this bespoke logic cannot +// handle every possible case (see tests marked with FIXMEs), but it gets +// the most common cases. +std::optional<std::tuple<std::string, std::string, std::string>> +extractTypeComponents(Preprocessor *PP, TypeLoc TL, const VarDecl *Var, + const ASTContext &Context) { + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + + SourceLocation EndLoc = getVarLocForReplacement(Var, Context); + if (!EndLoc.isValid()) + return std::nullopt; + std::optional<SmallVector<ClassifiedToken, 8>> MaybeTokens = + utils::type::classifyDeclTypeTokens(*PP, Var->getBeginLoc(), EndLoc, + Context); + if (!MaybeTokens || MaybeTokens->size() == 0) + return std::nullopt; + + unsigned LastTypeIndex = -1; + for (auto [Index, Token] : llvm::enumerate(*MaybeTokens)) { + if (!Token.IsSpecifier && !Token.IsQualifier) + LastTypeIndex = Index; + } + + struct TokenAndText { + ClassifiedToken T; + std::string Text; + + TokenAndText(ClassifiedToken T, StringRef Text) : T(T), Text(Text) {} + }; + + std::string Specifiers; + std::string QualType; + std::string BaseType; + std::string Suffix; + + SmallVector<TokenAndText, 8> Tokens; + for (auto [Index, Current] : llvm::enumerate(*MaybeTokens)) { + if (Index == MaybeTokens->size() - 1) { + if (Index == LastTypeIndex) { + CharSourceRange CSR{SourceRange{Current.T.getLocation()}, true}; + Tokens.emplace_back(Current, Lexer::getSourceText(CSR, SM, LangOpts)); + + SourceLocation SuffixStart = + findEndOfToken(Current.T.getLocation(), Context); + Suffix = Lexer::getSourceText( + CharSourceRange{SourceRange{SuffixStart, EndLoc}, false}, SM, + LangOpts); + } else { + CharSourceRange CSR{SourceRange{Current.T.getLocation(), EndLoc}, + false}; + Tokens.emplace_back(Current, Lexer::getSourceText(CSR, SM, LangOpts)); + } + } else { + const ClassifiedToken &Next = (*MaybeTokens)[Index + 1]; + if (Index == LastTypeIndex) { + Tokens.emplace_back( + Current, + Lexer::getSourceText( + CharSourceRange{SourceRange{Current.T.getLocation()}, true}, SM, + LangOpts)); + + SourceLocation SuffixStart = + findEndOfToken(Current.T.getLocation(), Context); + Suffix = Lexer::getSourceText( + CharSourceRange{SourceRange{SuffixStart, Next.T.getLocation()}, + false}, + SM, LangOpts); + } else { + Tokens.emplace_back( + Current, Lexer::getSourceText( + CharSourceRange{SourceRange{Current.T.getLocation(), + Next.T.getLocation()}, + false}, + SM, LangOpts)); + } + } + } + + // Track whether we've seen a '*' token while scanning right to left. + bool ConsumedStar = false; + std::optional<SourceLocation> StarLoc; + if (PointerTypeLoc PTL = TL.getUnqualifiedLoc().getAs<PointerTypeLoc>()) + StarLoc.emplace(PTL.getStarLoc()); + + // Track if we correctly found the CV qualifiers of the array. After + // processing the tokens, validate our assumptions and bail out if + // any assumption is not correct. + bool SawConst = false; + bool SawVolatile = false; + + // TypeLoc'c SourceLocation starts at the first non-qualifier, so any + // associated type qualifier is not included. However, type qualifiers + // may be embeded in the type ('unsigned const int'). + auto WithinTypeRange = [&, UnqualifiedTL = TL.getUnqualifiedLoc()]( + const ClassifiedToken &Token) -> bool { + if (!Token.IsQualifier) + return false; + + FullSourceLoc TokLoc{Token.T.getLocation(), SM}; + return FullSourceLoc(UnqualifiedTL.getBeginLoc(), SM) + .isBeforeInTranslationUnitThan(TokLoc) && + TokLoc.isBeforeInTranslationUnitThan(UnqualifiedTL.getEndLoc()); + }; + auto IsSpec = [&](const ClassifiedToken &Token) -> bool { + if (WithinTypeRange(Token)) + return false; + + if (ConsumedStar) { + return Token.IsSpecifier; + } else { + return Token.IsSpecifier || Token.IsQualifier; + } + }; + auto AppendTypeToken = [&](const ClassifiedToken &Token, StringRef Text) { + if (StarLoc.has_value() && Token.T.getLocation() == *StarLoc) { + ConsumedStar = true; + } + + if (!WithinTypeRange(Token) && !ConsumedStar) { + if (Token.T.is(tok::kw_const)) + SawConst = true; + if (Token.T.is(tok::kw_volatile)) + SawVolatile = true; + } + + if (IsSpec(Token)) { + Specifiers = (Text + Specifiers).str(); + } else { + if (ConsumedStar) { + QualType = (Text + QualType).str(); + } else { + BaseType = (Text + BaseType).str(); + } + } + }; + for (const auto &T : llvm::reverse(Tokens)) + AppendTypeToken(T.T, T.Text); + + if (ConsumedStar) { + Specifiers += BaseType; + } else { + assert(QualType.size() == 0); + QualType = BaseType; + } + + if (TL.getType().isLocalVolatileQualified() ^ SawVolatile) + return {}; + if (TL.getType().isLocalConstQualified() ^ SawConst) + return {}; + + return std::make_tuple(std::move(Specifiers), std::move(QualType), + std::move(Suffix)); +} + +bool AvoidCArraysCheck::replaceDecl(ArrayTypeLoc ATL, const VarDecl *Var, + bool UseCTAD, ASTContext &Context, + std::vector<FixItHint> &FixIts) { + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + + std::optional<std::tuple<std::string, std::string, std::string>> + TypeComponents = + extractTypeComponents(PP, ATL.getElementLoc(), Var, Context); + if (!TypeComponents) + return {}; + auto [Specifiers, QualType, Suffix] = *TypeComponents; + + if (UseCTAD && Suffix.empty()) + Suffix = " "; + + std::string Replacement; + const bool IsVLA = ATL.getTypePtr()->isVariableArrayType(); + const bool IsCharElem = + ATL.getElementLoc().getTypePtr()->isAnyCharacterType(); + if (UseCTAD) { + if (IsVLA) { + // FIXME - support vector + return false; + } else { + Replacement += Specifiers + "std::array" + Suffix; + } + } else { + if (IsVLA) { + // FIXME - support vector + return false; + } else { + Replacement += Specifiers + "std::array<" + QualType + ", "; + } + if (ATL.getSizeExpr()) { + Replacement += Lexer::getSourceText( + CharSourceRange::getCharRange( + ATL.getLBracketLoc().getLocWithOffset(1), ATL.getRBracketLoc()), + SM, LangOpts); + } else { + if (!Var->hasInit()) + return false; + + const Expr *InitExpr = Var->getInit(); + if (const auto *EWC = dyn_cast<ExprWithCleanups>(InitExpr)) + InitExpr = EWC->getSubExpr(); + + if (const auto *ILE = dyn_cast<InitListExpr>(InitExpr)) { + if (IsCharElem) { + if (ILE->getNumInits() != 1) + return false; + const Expr *FirstInitExpr = ILE->getInit(0); + if (const auto *SE = dyn_cast<StringLiteral>(FirstInitExpr)) + Replacement += std::to_string(SE->getLength() + 1); + else + return false; + } else { + Replacement += std::to_string(ILE->getNumInits()); + } + } else if (const auto *SE = dyn_cast<StringLiteral>(InitExpr)) { + Replacement += std::to_string(SE->getLength() + 1); + } else { + return false; + } + } + Replacement += ">" + Suffix; + } + + SourceLocation EndLoc = getVarLocForReplacement(Var, Context); + CharSourceRange ReplacementRange = + CharSourceRange::getCharRange(Var->getBeginLoc(), EndLoc); + + FixIts.push_back(FixItHint::CreateReplacement(ReplacementRange, Replacement)); + FixIts.push_back(FixItHint::CreateRemoval(ATL.getBracketsRange())); + return true; +} + +bool AvoidCArraysCheck::replaceArrayReferences(const VarDecl *Var, + const FunctionDecl *Func, + ASTContext &Context, + std::vector<FixItHint> &FixIts) { + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + + auto NonParensExpr = [](auto &&...Matchers) { + return expr(std::forward<decltype(Matchers)>(Matchers)..., + unless(parenExpr())); + }; + SmallVector<BoundNodes, 4> Matches = match( + findAll(declRefExpr( + to(equalsNode(Var)), unless(hasAncestor(typeAliasDecl())), + optionally(hasParent(varDecl(hasParent(declStmt( + hasParent(cxxForRangeStmt().bind("range-for"))))))), + optionally(hasAncestor( + NonParensExpr( + optionally(unaryExprOrTypeTraitExpr().bind("sizeof")), + optionally( + hasAncestor(NonParensExpr().bind("parent2Expr")))) + .bind("parent1")))) + .bind("ref")), + *Func->getBody(), Context); + for (const BoundNodes &Match : Matches) { + const DeclRefExpr *Ref = Match.getNodeAs<DeclRefExpr>("ref"); + if (Match.getNodeAs<CXXForRangeStmt>("range-for")) + continue; + if (Match.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof")) + continue; + + const Expr *Parent1 = Match.getNodeAs<Expr>("parent1"); + const Expr *Parent2Expr = Match.getNodeAs<Expr>("parent2Expr"); + if (!Parent1) + return false; + + if (const auto *Cast = dyn_cast<ImplicitCastExpr>(Parent1)) { + if (Cast->getCastKind() != CK_ArrayToPointerDecay) + return false; + if (const auto *Subscript = + dyn_cast_or_null<ArraySubscriptExpr>(Parent2Expr)) { + if (Subscript->getLHS() == Parent1) { + // Nothing to change in this case, e.g., 'arr[1]' + continue; + } else { + // Swap order of subscript, e.g., '1[arr]' + const bool NotTokenRange = true; + CharSourceRange NameRange{ + SourceRange{Subscript->getBase()->getBeginLoc(), + Subscript->getBase()->getEndLoc()}, + NotTokenRange}; + StringRef Name = Lexer::getSourceText(NameRange, SM, LangOpts); + CharSourceRange IdxRange{ + SourceRange{Subscript->getIdx()->getBeginLoc(), + Subscript->getIdx()->getEndLoc()}, + NotTokenRange}; + StringRef Idx = Lexer::getSourceText(IdxRange, SM, LangOpts); + FixIts.push_back(FixItHint::CreateReplacement(NameRange, Idx)); + FixIts.push_back(FixItHint::CreateReplacement(IdxRange, Name)); + } + } else { + const bool IsTokenRange = true; + if (Ref->getBeginLoc().isMacroID()) + return false; + + CharSourceRange NameRange{ + SourceRange{Ref->getBeginLoc(), Ref->getEndLoc()}, IsTokenRange}; + StringRef Name = Lexer::getSourceText(NameRange, SM, LangOpts); + FixIts.push_back( + FixItHint::CreateReplacement(NameRange, (Name + ".begin()").str())); + continue; + } + } else + return false; + } + + return true; +} + +std::vector<FixItHint> +AvoidCArraysCheck::replaceArray(ArrayTypeLoc ATL, const DeclStmt *VarDeclStmt, + const VarDecl *Var, ASTContext &Context) { + const FunctionDecl *Func = dyn_cast<FunctionDecl>(Var->getDeclContext()); + if (!Func || !Func->getBody()) + return {}; + + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + + const Type *ElemType = ATL.getElementLoc().getTypePtr(); + if (ElemType->isArrayType() || ElemType->isFunctionPointerType() || + ElemType->isMemberFunctionPointerType()) + return {}; + + std::vector<FixItHint> FixIts; + if (!replaceArrayReferences(Var, Func, Context, FixIts)) + return {}; + + if (!VarDeclStmt->isSingleDecl()) + return {}; + + const Expr *InitExpr = Var->hasInit() ? Var->getInit() : nullptr; + if (const auto *EWC = dyn_cast_or_null<ExprWithCleanups>(InitExpr)) + InitExpr = EWC->getSubExpr(); + + // Determine if we can use a CTAD declaration + const bool IsCharElem = + ATL.getElementLoc().getTypePtr()->isAnyCharacterType(); + bool UseCTAD = false; + if (getLangOpts().CPlusPlus17 && InitExpr && !ATL.getSizeExpr() && + !IsCharElem) { + if (const auto *ILE = dyn_cast<InitListExpr>(Var->getInit())) { + if (ILE->getNumInits() > 0) { + UseCTAD = llvm::all_of( + ILE->inits(), + [ElemType = + ATL.getElementLoc().getType().getTypePtr()](const Expr *E) { + const Expr *SpelledExpr = E->IgnoreUnlessSpelledInSource(); + if (dyn_cast<InitListExpr>(SpelledExpr)) + return false; + const auto *ConstructExpr = + dyn_cast<CXXConstructExpr>(SpelledExpr); + if (ConstructExpr && + !dyn_cast<CXXTemporaryObjectExpr>(SpelledExpr) && + ConstructExpr->isListInitialization()) + return false; + return SpelledExpr->getType().getTypePtr() == ElemType; + }); + } + } + } + + // Add FixIt {} around initializer + if (InitExpr && !UseCTAD) { + SourceRange InitRange; + if (const auto *SE = dyn_cast<StringLiteral>(InitExpr)) { + std::optional<Token> NextToken = + Lexer::findNextToken(SE->getEndLoc(), SM, LangOpts); + if (!NextToken) + return {}; + InitRange = SourceRange{SE->getBeginLoc(), NextToken->getLocation()}; + } else if (const auto *ILE = dyn_cast<InitListExpr>(InitExpr)) { + std::optional<Token> NextToken = + Lexer::findNextToken(ILE->getEndLoc(), SM, LangOpts); + if (!NextToken) + return {}; + InitRange = SourceRange{ILE->getBeginLoc(), NextToken->getLocation()}; + } + + if (InitRange.isValid()) { + if (utils::lexer::rangeContainsExpansionsOrDirectives(InitRange, SM, + LangOpts)) + return {}; + // TODO: How can I get FileCheck to accept '{{}}' as a non-regex match? + FixIts.push_back(FixItHint::CreateInsertion(InitRange.getBegin(), "{ ")); + FixIts.push_back(FixItHint::CreateInsertion(InitRange.getEnd(), " }")); + } + } + + if (!replaceDecl(ATL, Var, UseCTAD, Context, FixIts)) + return {}; + + std::optional<FixItHint> IncludeFixIt = + IncludeInserter.createIncludeInsertion(SM.getFileID(Var->getBeginLoc()), + "<array>"); + if (IncludeFixIt) + FixIts.push_back(std::move(*IncludeFixIt)); + + return FixIts; +} + void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) { const auto *ArrayType = Result.Nodes.getNodeAs<TypeLoc>("typeloc"); - diag(ArrayType->getBeginLoc(), - "do not declare %select{C-style|C VLA}0 arrays, use " - "%select{std::array<>|std::vector<>}0 instead") - << ArrayType->getTypePtr()->isVariableArrayType(); + bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType(); + auto Diag = diag(ArrayType->getBeginLoc(), + "do not declare %select{C-style|C VLA}0 arrays, use " + "%select{std::array<>|std::vector<>}0 instead") + << IsVLA; + + const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var"); + const auto *VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl"); + if (!Var || !VarDeclStmt) + return; + + // FIXME: Support variables with attributes + if (Var->hasAttrs()) + return; + + ArrayTypeLoc ATL = ArrayType->getUnqualifiedLoc().getAs<ArrayTypeLoc>(); + if (!ATL) + return; + Diag << replaceArray(ATL, VarDeclStmt, Var, *Result.Context); +} + +void AvoidCArraysCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + this->PP = PP; + IncludeInserter.registerPreprocessor(PP); +} + +void AvoidCArraysCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); } +AvoidCArraysCheck::AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} + } // namespace clang::tidy::modernize
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits