[cfe-dev] A doubt about cvr-qualified array types.

[Eli Friedman]

On Sun, Oct 24, 2010 at 2:19 PM, Enea Zaffanella <zaffanella at cs.unipr.it> wrote:
> Il 24/10/2010 21:08, Eli Friedman ha scritto:
>> On Sun, Oct 24, 2010 at 11:03 AM, Abramo Bagnara
>> <abramo.bagnara at gmail.com> wrote:
>>> Il 23/10/2010 20:45, Eli Friedman ha scritto:
>>>> On Sat, Oct 23, 2010 at 10:29 AM, Abramo Bagnara
>>>> <abramo.bagnara at gmail.com> wrote:
>>>>> Il 23/10/2010 19:09, Eli Friedman ha scritto:
>>>>>> On Sat, Oct 23, 2010 at 2:15 AM, Enea Zaffanella <zaffanella at cs.unipr.it> wrote:
>>>>>>> Hello.
>>>>>>>
>>>>>>> We have a doubt regarding qualified array types.
>
> [...snip...]
>
>> In terms of putting const qualifiers on the array type itself, it's
>> mostly just efficiency; it saves creating an extra type, and it
>> doesn't really make anything much more complicated than we already
>> have to be able to deal with.
>>
>>>>> Does this means that we might encounter all the possible combinations
>>>>> (i.e. element with qualified/unqualified type inside nested arrays with
>>>>> qualified/unqualified type) and that the qualifiers of arrays types and
>>>>> item type should be merged to get a "normalized" type (not canonical)
>>>>> where qualifiers are pushed to the item type as standard mandates?
>>>>>
>>>>> There is a method to get such type?
>>>>
>>>> Why would you need such a type?
>>>
>>> Consider this example:
>>>
>>> struct S {
>>>  volatile int a[2];
>>> };
>>>
>>> void foo(void) {
>>>  const struct S* ps;
>>>  &ps->a;
>>> }
>>>
>>> The type of &ps->a has a volatile qualifier attached to builtin int type
>>> and a const qualifier attached to array type.
>>>
>>> It seems to me rather obvious that this is an undesiderable situation
>>> for library client that ask a sensible type for an Expr AST node.
>>>
>>> Note also that methods like QualType::getCVRQualifiersThroughArrayTypes
>>> does not handle properly this situation and obtain the wrong result.
>>>
>>> To tell you the truth I don't see many reasons to synthesize array Types
>>> with qualifiers instead to push the qualifiers in the right place, but
>>> probably I'm missing something...
>>
>> Assuming the APIs aren't buggy, it should essentially be invisible for
>> code that isn't digging through QualType internals...
>
> I am not sure I fully understand the motivations for NOT always pushing
> the qualifiers down to the element type.
>
> On the one hand, I really doubt there can actually be a (significant)
> efficiency penalty.
>
> On the other hand, having the element-type qualifiers potentially stored
> in many places is going to be troublesome for end-user applications. It
> would really be of great help if these qualifiers are always found in
> their correct place.

The primary issue is that if we unconditionally push down all
qualifiers all the way to the element type, we lose "sugar" (typedefs
etc.).  So the qualifiers can't always be in the correct place.
Putting them in the correct place some of the time doesn't seem like
it helps... the TypePrinter example is really an edge case because
it's not working with the semantics of the types in question.

> Note that canonicalization is often a solution, but it is not _always_
> the solution. Sometimes we do not want to canonicalize types, because
> expanding all the typedefs reduces readability.
>
> Are you saying that all clang-based applications working at the
> source-code level should search and go through all of the
> (multidimensional) array-type derivations and explicitly collect all of
> the type qualifiers in order to produce a correctly qualified element
> type? This sounds rather error-prone.

It's tricky if you have to write the code yourself, but applications
can just call ASTContext::getBaseElementType to get a correctly
qualified element type.

>> but as you
>> correctly point out, there are bugs.  I think the right way to
>> approach it is to just fix the bugs, though.
>>
>> -Eli
>
> As mentioned above, besides the potential bugs inside clang, it is also
> worth considering whether such an implementation choice is going to
> increase or decrease the rate of bugs in the applications that are based
> on clang.

Because of the issues mentioned above, I really don't think it will help.

> Anyway, here is some code that is accepted by gcc and rejected by clang
> and is related to qualifications of array types.
> I am not sure whether or not it is a genuine bug ... but according to my
> own reading of 6.7.3p2 and 6.7.3p8 it seems to be legal code.
>
> $ cat restricted_array.c
> typedef int* Array[10];
> typedef __restrict__ Array RArray;
>
> $ llvm/Debug+Asserts/bin/clang -cc1 restricted_array.c
> restricted_array.c:3:9: error: restrict requires a pointer or reference
> ('Array' (aka 'int *[10]') is invalid)
> typedef __restrict__ Array RArray;
> ~~~~~~~~^~~~~~~~~~~~~~~~~~
> 1 error generated.

Err, wow... that's pretty wacky.  Comeau actually agrees with clang,
but I don't know if that's intentional.  Either way, though, the
restrict qualified type never actually exists in the AST, so it's not
really relevant to this discussion.

-Eli