[clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 15 13:52:14 PDT 2021


On Tue, Sep 14, 2021 at 10:04 AM Richard Smith <richard at metafoo.co.uk>
wrote:

> On Mon, 13 Sept 2021 at 19:24, David Blaikie via cfe-commits <
> cfe-commits at 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]".

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]"?



>
>
>>        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 at -1{{static_assert failed due to requirement
>> 'static_cast<const X<int> *>(nullptr)'}}
>>    static_assert((const X<typename T::T>[]){} == nullptr);
>> -  // expected-error at -1{{static_assert failed due to requirement
>> '(X<int> const[0]){} == nullptr'}}
>> +  // expected-error at -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 at -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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210915/6d42852f/attachment-0001.html>


More information about the cfe-commits mailing list