Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
On Thu, Oct 14, 2021 at 2:25 PM David Blaikie wrote:
> On Tue, Oct 12, 2021 at 7:35 PM David Blaikie wrote:
>
>> On Mon, Oct 11, 2021 at 2:46 PM Richard Smith
>> wrote:
>>
>>> On Wed, 15 Sept 2021 at 13:52, David Blaikie wrote:
>>>
On Tue, Sep 14, 2021 at 10:04 AM Richard Smith
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(T))
>> -TC = AT->desugar()->getTypeClass();
>> +UnderlyingType = AT->desugar().getTypePtr();
>>if (const auto *Subst = dyn_cast(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(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:
>>>
>>> :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, right, good to understand! Yep, fixed and tested with this:
>> 39093279f2ede4af9048b89d048d7fe9182a50f8
>>
>> Huh, funny diagnostic experience aside here:
>> https://godbolt.org/z/W46b3qc49
>> void f(int i) {
>> typedef int x[i];
>> const x y = {};
>> }
>> :2:19: error: variable-sized object may not be initialized
>> typedef int x[i];
>> ^
>>
>> With no pointer/note/etc to
Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
On Tue, Oct 12, 2021 at 7:35 PM David Blaikie wrote:
> On Mon, Oct 11, 2021 at 2:46 PM Richard Smith
> wrote:
>
>> On Wed, 15 Sept 2021 at 13:52, David Blaikie wrote:
>>
>>> On Tue, Sep 14, 2021 at 10:04 AM Richard Smith
>>> 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(T))
> -TC = AT->desugar()->getTypeClass();
> +UnderlyingType = AT->desugar().getTypePtr();
>if (const auto *Subst = dyn_cast(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(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:
>>
>> :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, right, good to understand! Yep, fixed and tested with this:
> 39093279f2ede4af9048b89d048d7fe9182a50f8
>
> Huh, funny diagnostic experience aside here:
> https://godbolt.org/z/W46b3qc49
> void f(int i) {
> typedef int x[i];
> const x y = {};
> }
> :2:19: error: variable-sized object may not be initialized
> typedef int x[i];
> ^
>
> With no pointer/note/etc to the 'y' variable - the typedef and variable
> could be quite far apart. The diagnostic would be hard to read/understand.
>
>
>> Oh, also - another quirk of array type printing that I'd be up for
>>>
Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
On Mon, Oct 11, 2021 at 2:46 PM Richard Smith wrote:
> On Wed, 15 Sept 2021 at 13:52, David Blaikie wrote:
>
>> On Tue, Sep 14, 2021 at 10:04 AM Richard Smith
>> 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(T))
-TC = AT->desugar()->getTypeClass();
+UnderlyingType = AT->desugar().getTypePtr();
if (const auto *Subst = dyn_cast(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(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:
>
> :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, right, good to understand! Yep, fixed and tested with this:
39093279f2ede4af9048b89d048d7fe9182a50f8
Huh, funny diagnostic experience aside here: https://godbolt.org/z/W46b3qc49
void f(int i) {
typedef int x[i];
const x y = {};
}
:2:19: error: variable-sized object may not be initialized
typedef int x[i];
^
With no pointer/note/etc to the 'y' variable - the typedef and variable
could be quite far apart. The diagnostic would be hard to read/understand.
> 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
Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
On Wed, 15 Sept 2021 at 13:52, David Blaikie wrote:
> On Tue, Sep 14, 2021 at 10:04 AM Richard Smith
> 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(T))
>>> -TC = AT->desugar()->getTypeClass();
>>> +UnderlyingType = AT->desugar().getTypePtr();
>>>if (const auto *Subst = dyn_cast(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(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:
: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
>>> +++
Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
Ping on this
On Wed, Sep 15, 2021 at 1:52 PM David Blaikie wrote:
>
>
> On Tue, Sep 14, 2021 at 10:04 AM Richard Smith
> 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(T))
>>> -TC = AT->desugar()->getTypeClass();
>>> +UnderlyingType = AT->desugar().getTypePtr();
>>>if (const auto *Subst = dyn_cast(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(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
>>> ---
Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
On Tue, Sep 14, 2021 at 10:04 AM Richard Smith
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(T))
>> -TC = AT->desugar()->getTypeClass();
>> +UnderlyingType = AT->desugar().getTypePtr();
>>if (const auto *Subst = dyn_cast(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(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.20e+01i
>>
>>constexpr _Complex int ArrayOfComplexInt[10]
Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
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(T))
> -TC = AT->desugar()->getTypeClass();
> +UnderlyingType = AT->desugar().getTypePtr();
>if (const auto *Subst = dyn_cast(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(UnderlyingType)->getElementType().getTypePtr(),
> + NeedARCStrongQualifier);
> case Type::VariableArray:
> case Type::DependentSizedArray:
>
Can we give these two cases the same treatment?
>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.20e+01i
>
>constexpr _Complex int ArrayOfComplexInt[10] = {ComplexInt, ComplexInt,
> ComplexInt, ComplexInt};
> - // CHECK: | `-VarDecl {{.*}} col:{{.*}}
> ArrayOfComplexInt '_Complex int const[10]' constexpr cinit
> + // CHECK: | `-VarDecl {{.*}} 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:{{.*}}
> ArrayOfComplexFloat '_Complex float const[10]' constexpr cinit
> + // CHECK:`-VarDecl {{.*}} col:{{.*}}
> ArrayOfComplexFloat 'const _Complex float [10]' constexpr cinit
>// CHECK-NEXT: |-value: Array size=10
>// CHECK-NEXT: | |-elements: ComplexFloat 3.141500e+00 +
> 4.20e+01i, ComplexFloat 3.141500e+00 + 4.20e+01i, ComplexFloat
> 4.20e+01 + 2.40e+01i, ComplexFloat 4.20e+01 + 2.40e+01i
>// CHECK-NEXT: | `-filler: 6 x ComplexFloat 0.00e+00 +
> 0.00e+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] = {
>
[clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array
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]).
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(T))
-TC = AT->desugar()->getTypeClass();
+UnderlyingType = AT->desugar().getTypePtr();
if (const auto *Subst = dyn_cast(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(UnderlyingType)->getElementType().getTypePtr(),
+ NeedARCStrongQualifier);
case Type::VariableArray:
case Type::DependentSizedArray:
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.20e+01i
constexpr _Complex int ArrayOfComplexInt[10] = {ComplexInt, ComplexInt,
ComplexInt, ComplexInt};
- // CHECK: | `-VarDecl {{.*}} col:{{.*}}
ArrayOfComplexInt '_Complex int const[10]' constexpr cinit
+ // CHECK: | `-VarDecl {{.*}} 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:{{.*}}
ArrayOfComplexFloat '_Complex float const[10]' constexpr cinit
+ // CHECK:`-VarDecl {{.*}} col:{{.*}}
ArrayOfComplexFloat 'const _Complex float [10]' constexpr cinit
// CHECK-NEXT: |-value: Array size=10
// CHECK-NEXT: | |-elements: ComplexFloat 3.141500e+00 + 4.20e+01i,
ComplexFloat 3.141500e+00 + 4.20e+01i, ComplexFloat 4.20e+01 +
2.40e+01i, ComplexFloat 4.20e+01 + 2.40e+01i
// CHECK-NEXT: | `-filler: 6 x ComplexFloat 0.00e+00 + 0.00e+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:{{.*}} arr_f
'float const[3][5]' constexpr cinit
+ // CHECK: | `-VarDecl {{.*}} 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.00e+00,
