On Wed, 15 Sept 2021 at 13:52, David Blaikie <dblai...@gmail.com> wrote:
> On Tue, Sep 14, 2021 at 10:04 AM Richard Smith <rich...@metafoo.co.uk> > wrote: > >> On Mon, 13 Sept 2021 at 19:24, David Blaikie via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> >>> Author: David Blaikie >>> Date: 2021-09-13T19:17:05-07:00 >>> New Revision: 2bd84938470bf2e337801faafb8a67710f46429d >>> >>> URL: >>> https://github.com/llvm/llvm-project/commit/2bd84938470bf2e337801faafb8a67710f46429d >>> DIFF: >>> https://github.com/llvm/llvm-project/commit/2bd84938470bf2e337801faafb8a67710f46429d.diff >>> >>> LOG: Improve type printing of const arrays to normalize array-of-const >>> and const-array >>> >>> Since these map to the same effective type - render them the same/in the >>> more legible way (const x[n]). >>> >> >> Nice! >> >> >>> Added: >>> >>> >>> Modified: >>> clang/lib/AST/TypePrinter.cpp >>> clang/test/ARCMT/cxx-checking.mm >>> clang/test/AST/ast-dump-APValue-arithmetic.cpp >>> clang/test/AST/ast-dump-APValue-array.cpp >>> clang/test/CXX/basic/basic.types/p10.cpp >>> clang/test/Sema/assign.c >>> clang/test/Sema/typedef-retain.c >>> clang/test/SemaCXX/reinterpret-cast.cpp >>> clang/test/SemaCXX/static-assert-cxx17.cpp >>> >>> Removed: >>> >>> >>> >>> >>> ################################################################################ >>> diff --git a/clang/lib/AST/TypePrinter.cpp >>> b/clang/lib/AST/TypePrinter.cpp >>> index aef1e4f3f4953..251db97c7db08 100644 >>> --- a/clang/lib/AST/TypePrinter.cpp >>> +++ b/clang/lib/AST/TypePrinter.cpp >>> @@ -200,11 +200,12 @@ bool TypePrinter::canPrefixQualifiers(const Type >>> *T, >>> // type expands to a simple string. >>> bool CanPrefixQualifiers = false; >>> NeedARCStrongQualifier = false; >>> - Type::TypeClass TC = T->getTypeClass(); >>> + const Type *UnderlyingType = T; >>> if (const auto *AT = dyn_cast<AutoType>(T)) >>> - TC = AT->desugar()->getTypeClass(); >>> + UnderlyingType = AT->desugar().getTypePtr(); >>> if (const auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) >>> - TC = Subst->getReplacementType()->getTypeClass(); >>> + UnderlyingType = Subst->getReplacementType().getTypePtr(); >>> + Type::TypeClass TC = UnderlyingType->getTypeClass(); >>> >>> switch (TC) { >>> case Type::Auto: >>> @@ -243,6 +244,9 @@ bool TypePrinter::canPrefixQualifiers(const Type *T, >>> >>> case Type::ConstantArray: >>> case Type::IncompleteArray: >>> + return canPrefixQualifiers( >>> + >>> cast<ArrayType>(UnderlyingType)->getElementType().getTypePtr(), >>> + NeedARCStrongQualifier); >>> case Type::VariableArray: >>> case Type::DependentSizedArray: >>> >> >> Can we give these two cases the same treatment? >> > > Handled the DependentSizedArray in > https://github.com/llvm/llvm-project/commit/40acc0adad59ac39e9a7a02fcd93161298500c00 > > But per the comment in that commit I wasn't able to reproduce the problem > with a variable array - though we could include it in the handling on > principle/for consistency, even without a test/etc. Perhaps there's a way > to test/provoke the behavior you might know that I couldn't figure out? > > Details from the commit: > > The VariableArray case I couldn't figure out how to test/provoke - you > > can't write/form a variable array in any context other than a local > > variable that I know of, and in that case `const int x[n]` is the > > normalized form already (array-of-const) and you can't use typedefs > > (since you can't typedef int[n] with variable 'n') to force the > > const-array AST that would produce the undesirable type printing "int > > const [n]". > C has a fairly surprising rule here -- you actually can define a VLA type in a local typedef. For example, void f(int n) { typedef int arr[n]; const arr x; } ... is valid in C. In C++ mode, this produces: <source>:3:15: error: default initialization of an object of const type 'const arr' (aka 'int const[n]') (note, 'int const[n]' not 'const int[n]'). > Oh, also - another quirk of array type printing that I'd be up for > addressing if you've got an opinion on it: > > "int [n]" is printed with a space between the "int" and array. > "int *const[n]" is printed without a space before the array (though both > have a space after "int") > > ClangFormat formats these as "int[n]" and "int *const[n]" - with * > left/right depending on ClangFormat's heuristics/configuration. > > Reckon we should remove the space in "int[n]"? > I think we should make it consistent, one way or the other. Beyond that, the decision seems somewhat arbitrary, but following clang-format seems reasonable to me. I certainly would expect `int *[n]` rather than `int * [n]`, which similarly suggests omitting the space here. NeedARCStrongQualifier = true; >> >> >>> diff --git a/clang/test/ARCMT/cxx-checking.mm b/clang/test/ARCMT/ >>> cxx-checking.mm >>> index 952f3cdcbb79c..d6441def09b40 100644 >>> --- a/clang/test/ARCMT/cxx-checking.mm >>> +++ b/clang/test/ARCMT/cxx-checking.mm >>> @@ -80,7 +80,7 @@ >>> >>> struct FlexibleArrayMember0 { >>> int length; >>> - id array[]; // expected-error{{flexible array member 'array' of type >>> 'id __strong[]' with non-trivial destruction}} >>> + id array[]; // expected-error{{flexible array member 'array' of type >>> '__strong id []' with non-trivial destruction}} >>> }; >>> >>> struct FlexibleArrayMember1 { >>> >>> diff --git a/clang/test/AST/ast-dump-APValue-arithmetic.cpp >>> b/clang/test/AST/ast-dump-APValue-arithmetic.cpp >>> index 835d8c8108346..e51c1cee04cfe 100644 >>> --- a/clang/test/AST/ast-dump-APValue-arithmetic.cpp >>> +++ b/clang/test/AST/ast-dump-APValue-arithmetic.cpp >>> @@ -36,13 +36,13 @@ void Test() { >>> // CHECK-NEXT: | |-value: ComplexFloat 3.141500e+00 + 4.200000e+01i >>> >>> constexpr _Complex int ArrayOfComplexInt[10] = {ComplexInt, >>> ComplexInt, ComplexInt, ComplexInt}; >>> - // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> ArrayOfComplexInt '_Complex int const[10]' constexpr cinit >>> + // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> ArrayOfComplexInt 'const _Complex int [10]' constexpr cinit >>> // CHECK-NEXT: | |-value: Array size=10 >>> // CHECK-NEXT: | | |-elements: ComplexInt 42 + 24i, ComplexInt 42 >>> + 24i, ComplexInt 42 + 24i, ComplexInt 42 + 24i >>> // CHECK-NEXT: | | `-filler: 6 x ComplexInt 0 + 0i >>> >>> constexpr _Complex float ArrayOfComplexFloat[10] = {ComplexFloat, >>> ComplexFloat, ComplexInt, ComplexInt}; >>> - // CHECK: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> ArrayOfComplexFloat '_Complex float const[10]' constexpr cinit >>> + // CHECK: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> ArrayOfComplexFloat 'const _Complex float [10]' constexpr cinit >>> // CHECK-NEXT: |-value: Array size=10 >>> // CHECK-NEXT: | |-elements: ComplexFloat 3.141500e+00 + >>> 4.200000e+01i, ComplexFloat 3.141500e+00 + 4.200000e+01i, ComplexFloat >>> 4.200000e+01 + 2.400000e+01i, ComplexFloat 4.200000e+01 + 2.400000e+01i >>> // CHECK-NEXT: | `-filler: 6 x ComplexFloat 0.000000e+00 + >>> 0.000000e+00i >>> >>> diff --git a/clang/test/AST/ast-dump-APValue-array.cpp >>> b/clang/test/AST/ast-dump-APValue-array.cpp >>> index f9b38ec332a5b..72e519208ac56 100644 >>> --- a/clang/test/AST/ast-dump-APValue-array.cpp >>> +++ b/clang/test/AST/ast-dump-APValue-array.cpp >>> @@ -43,7 +43,7 @@ void Test() { >>> constexpr float arr_f[3][5] = { >>> {1, 2, 3, 4, 5}, >>> }; >>> - // CHECK: | `-VarDecl {{.*}} <line:{{.*}}, line:{{.*}}> line:{{.*}} >>> arr_f 'float const[3][5]' constexpr cinit >>> + // CHECK: | `-VarDecl {{.*}} <line:{{.*}}, line:{{.*}}> line:{{.*}} >>> arr_f 'const float [3][5]' constexpr cinit >>> // CHECK-NEXT: | |-value: Array size=3 >>> // CHECK-NEXT: | | |-element: Array size=5 >>> // CHECK-NEXT: | | | |-elements: Float 1.000000e+00, Float >>> 2.000000e+00, Float 3.000000e+00, Float 4.000000e+00 >>> @@ -52,7 +52,7 @@ void Test() { >>> // CHECK-NEXT: | | `-filler: 5 x Float 0.000000e+00 >>> >>> constexpr S0 arr_s0[2] = {{1, 2}, {3, 4}}; >>> - // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> arr_s0 'S0 const[2]' constexpr cinit >>> + // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> arr_s0 'const S0 [2]' constexpr cinit >>> // CHECK-NEXT: | |-value: Array size=2 >>> // CHECK-NEXT: | | |-element: Struct >>> // CHECK-NEXT: | | | `-field: Array size=2 >>> @@ -62,12 +62,12 @@ void Test() { >>> // CHECK-NEXT: | | `-elements: Int 3, Int 4 >>> >>> constexpr U0 arr_u0[2] = {{.i = 42}, {.f = 3.1415f}}; >>> - // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> arr_u0 'U0 const[2]' constexpr cinit >>> + // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> arr_u0 'const U0 [2]' constexpr cinit >>> // CHECK-NEXT: | |-value: Array size=2 >>> // CHECK-NEXT: | | `-elements: Union .i Int 42, Union .f Float >>> 3.141500e+00 >>> >>> constexpr S1 arr_s1[2] = {}; >>> - // CHECK: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> arr_s1 'S1 const[2]' constexpr cinit >>> + // CHECK: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} >>> arr_s1 'const S1 [2]' constexpr cinit >>> // CHECK-NEXT: |-value: Array size=2 >>> // CHECK-NEXT: | |-element: Struct >>> // CHECK-NEXT: | | |-field: Struct >>> >>> diff --git a/clang/test/CXX/basic/basic.types/p10.cpp >>> b/clang/test/CXX/basic/basic.types/p10.cpp >>> index aac07c76bed2e..eae0c6513ac24 100644 >>> --- a/clang/test/CXX/basic/basic.types/p10.cpp >>> +++ b/clang/test/CXX/basic/basic.types/p10.cpp >>> @@ -32,7 +32,7 @@ struct Incomplete; // expected-note 2{{forward >>> declaration of 'Incomplete'}} >>> template<class T> struct ClassTemp {}; >>> >>> constexpr Incomplete incomplete = {}; // expected-error {{constexpr >>> variable cannot have non-literal type 'const Incomplete'}} expected-note >>> {{incomplete type 'const Incomplete' is not a literal type}} >>> -constexpr Incomplete incomplete2[] = {}; // expected-error {{constexpr >>> variable cannot have non-literal type 'Incomplete const[]'}} expected-note >>> {{incomplete type 'Incomplete const[]' is not a literal type}} >>> +constexpr Incomplete incomplete2[] = {}; // expected-error {{constexpr >>> variable cannot have non-literal type 'const Incomplete []'}} expected-note >>> {{incomplete type 'const Incomplete []' is not a literal type}} >>> constexpr ClassTemp<int> classtemplate = {}; >>> constexpr ClassTemp<int> classtemplate2[] = {}; >>> >>> >>> diff --git a/clang/test/Sema/assign.c b/clang/test/Sema/assign.c >>> index 3edd5d4feaf2e..d65f9bf26547e 100644 >>> --- a/clang/test/Sema/assign.c >>> +++ b/clang/test/Sema/assign.c >>> @@ -13,7 +13,7 @@ typedef int arr[10]; >>> void test3() { >>> const arr b; // expected-note {{variable 'b' declared const >>> here}} >>> const int b2[10]; // expected-note {{variable 'b2' declared const >>> here}} >>> - b[4] = 1; // expected-error {{cannot assign to variable 'b' >>> with const-qualified type 'const arr' (aka 'int const[10]')}} >>> + b[4] = 1; // expected-error {{cannot assign to variable 'b' >>> with const-qualified type 'const arr' (aka 'const int [10]')}} >>> b2[4] = 1; // expected-error {{cannot assign to variable 'b2' >>> with const-qualified type 'const int [10]'}} >>> } >>> >>> >>> diff --git a/clang/test/Sema/typedef-retain.c >>> b/clang/test/Sema/typedef-retain.c >>> index 70e2abc1da5b8..d4358bce5ee5b 100644 >>> --- a/clang/test/Sema/typedef-retain.c >>> +++ b/clang/test/Sema/typedef-retain.c >>> @@ -17,7 +17,7 @@ typedef int a[5]; >>> void test3() { >>> typedef const a b; >>> b r; // expected-note {{variable 'r' declared const here}} >>> - r[0] = 10; // expected-error {{cannot assign to variable 'r' with >>> const-qualified type 'b' (aka 'int const[5]')}} >>> + r[0] = 10; // expected-error {{cannot assign to variable 'r' with >>> const-qualified type 'b' (aka 'const int [5]')}} >>> } >>> >>> int test4(const a y) { >>> >>> diff --git a/clang/test/SemaCXX/reinterpret-cast.cpp >>> b/clang/test/SemaCXX/reinterpret-cast.cpp >>> index 6a4bc91665448..f427af1efbafc 100644 >>> --- a/clang/test/SemaCXX/reinterpret-cast.cpp >>> +++ b/clang/test/SemaCXX/reinterpret-cast.cpp >>> @@ -132,7 +132,7 @@ void const_arrays() { >>> const STRING *s; >>> const char *c; >>> >>> - (void)reinterpret_cast<char *>(s); // expected-error >>> {{reinterpret_cast from 'const STRING *' (aka 'char const (*)[10]') to >>> 'char *' casts away qualifiers}} >>> + (void)reinterpret_cast<char *>(s); // expected-error >>> {{reinterpret_cast from 'const STRING *' (aka 'const char (*)[10]') to >>> 'char *' casts away qualifiers}} >>> (void)reinterpret_cast<const STRING *>(c); >>> } >>> >>> >>> diff --git a/clang/test/SemaCXX/static-assert-cxx17.cpp >>> b/clang/test/SemaCXX/static-assert-cxx17.cpp >>> index fe02f8f29965f..d5601839bdd3e 100644 >>> --- a/clang/test/SemaCXX/static-assert-cxx17.cpp >>> +++ b/clang/test/SemaCXX/static-assert-cxx17.cpp >>> @@ -94,7 +94,7 @@ void foo6() { >>> static_assert(static_cast<const X<typename T::T> *>(nullptr)); >>> // expected-error@-1{{static_assert failed due to requirement >>> 'static_cast<const X<int> *>(nullptr)'}} >>> static_assert((const X<typename T::T>[]){} == nullptr); >>> - // expected-error@-1{{static_assert failed due to requirement >>> '(X<int> const[0]){} == nullptr'}} >>> + // expected-error@-1{{static_assert failed due to requirement >>> '(const X<int> [0]){} == nullptr'}} >>> static_assert(sizeof(X<decltype(X<typename T::T>().X<typename >>> T::T>::~X())>) == 0); >>> // expected-error@-1{{static_assert failed due to requirement >>> 'sizeof(X<void>) == 0'}} >>> static_assert(constexpr_return_false<typename T::T, typename T::U>()); >>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits