[cfe-dev] ArrayType and qualifiers

[John McCall]

On Jan 27, 2011, at 3:33 PM, Abramo Bagnara wrote:
>>> 1) why element qualifiers are *moved* to ArrayType instead of to be
>>> *propagated* to ArrayType? (i.e. qualifiers would be in both ArrayType
>>> and element type)
>> 
>> Because it's still quite expensive to canonicalize a type in that representation,
>> and it becomes very awkward to manipulate qualifiers on even canonical
>> types.
> 
> I'm not sure to follow you: canonicalization would not become more CPU
> expensive with that representation, we've tried the patch and it is trivial.

Right now, canonicalizing a type never requires rebuilding types, which it used to.  Any patch which uses your proposed representation has to rebuild types when adding qualifiers to an array type, which is quite possible.  If your trivial patch doesn't do this, it is broken.

Your patch probably assumes that we only ever add and remove qualifiers in ways that mirror the patterns of how they can be legally written in declarators, which is simply not correct.

> void f() {
>  const int (*x)[1] = 0;
>  int (*y)[1] = 0;
>  x = y;
> }
> 
> Note that however also with current svn trunk we have a problem with
> this test: the assignment x = y should be compiled without warning in
> C++ *but* it's an assignment between incompatible pointers in C (clang
> currently does not emit any warning on both C and C++).

It's not an assignment between incompatible pointers in C.  y is a T*, where T = int[1];  converting that to the type of x (const T*) is a 6.3.2.3p2 pointer conversion.

You are probably thinking of the C++ rule governing indirect pointer conversions (T** can be converted to T cv * const *), but that is not the rule in effect here.

>>> 2) considered that now calling getElementType() on a canonical ArrayType
>>> returns the wrong result, how is possible to get the right element type
>>> e.g. using TypeVisitor?
>> 
>> I hadn't really considered the effects this would have on people using
>> TypeVisitor on a canonical type.  Mostly, I wasn't aware that that was at all
>> common.  If it is, maybe we should change TypeVisitor.
> 
> I don't understand how this might be feasible: we enter
> TypeVisitor::Visit with a Type* and as a consequence in the ArrayType
> dispatcher we have already lost any info about qualifier that should be
> applied to element type. The fact is that currently a canonical
> ArrayType has not enough info to know the qualifiers of its element, I
> believe this is an unfortunate situation.

This would obviously have to be done at the level of TypeVisitor::Visit(QualType).

That brings up a good point, actually — Visit(const Type*) is already stripping qualifiers off of every other type, so I'm not sure why I'm supposed to be particularly bothered that it strips the qualifiers off array types.

> Note also that to increase further the confusion, the API has also
> QualType ASTContext::getBaseElementType(const ArrayType *VAT) const...
> it's a mystery for me how this might work correctly. I'd hope it is
> never called ;-)

The expectation is that the user passes in something they acquired from ASTContext::getAsArrayType, which does this (non-canonical) pushing down of qualifiers.

John.