[cfe-commits] r92318 - /cfe/trunk/lib/AST/ASTContext.cpp

[Chandler Carruth]

On Mon, Jan 4, 2010 at 10:36 PM, Zhongxing Xu <xuzhongxing at gmail.com> wrote:
>
>
> 2010/1/5 Chris Lattner <clattner at apple.com>
>>
>> On Dec 30, 2009, at 2:59 PM, Zhongxing Xu wrote:
>>
>> > Author: zhongxingxu
>> > Date: Wed Dec 30 16:59:54 2009
>> > New Revision: 92318
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=92318&view=rev
>> > Log:
>> > The element type should also be canonicalized. Add a case for
>> > VariableArrayType.
>>
>> Hi Zhongxing,
>>
>> I don't think that this patch is correct.  According to the comment (which
>> I moved to the .cpp file):
>>
>>  /// \brief Returns this type as a completely-unqualified array type,
>> capturing
>>  /// the qualifiers in Quals. This only operates on canonical types in
>> order
>>  /// to ensure the ArrayType doesn't itself have qualifiers.
>>
>> I think it would be better to assert that the type is canonical on entry
>> to the method.
>
> Sorry, I didn't quite understand your point. We are already asserting the
> QualType T is canonical on the entry to the method.
>
> The patch makes sure the element type is canonical when we pass it to the
> method getUnqualifiedArrayType().

I think what Chris is getting at is that the element type should
already be canonical if the array type is canonical (by my reading of
getCanonicalType)?

>
>>
>> -Chris
>>
>> >
>> > Modified:
>> >    cfe/trunk/lib/AST/ASTContext.cpp
>> >
>> > Modified: cfe/trunk/lib/AST/ASTContext.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=92318&r1=92317&r2=92318&view=diff
>> >
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/AST/ASTContext.cpp (original)
>> > +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Dec 30 16:59:54 2009
>> > @@ -2383,7 +2383,7 @@
>> >   assert(!T.hasQualifiers() && "canonical array type has qualifiers!");
>> >   const ArrayType *AT = cast<ArrayType>(T);
>> >   QualType Elt = AT->getElementType();
>> > -  QualType UnqualElt = getUnqualifiedArrayType(Elt, Quals);
>> > +  QualType UnqualElt = getUnqualifiedArrayType(getCanonicalType(Elt),
>> > Quals);
>> >   if (Elt == UnqualElt)
>> >     return T;
>> >
>> > @@ -2396,6 +2396,12 @@
>> >     return getIncompleteArrayType(UnqualElt, IAT->getSizeModifier(), 0);
>> >   }
>> >
>> > +  if (const VariableArrayType *VAT = dyn_cast<VariableArrayType>(T)) {
>> > +    return getVariableArrayType(UnqualElt,
>> > VAT->getSizeExpr()->Retain(),
>> > +                                VAT->getSizeModifier(), 0,
>> > +                                SourceRange());
>> > +  }
>> > +
>> >   const DependentSizedArrayType *DSAT =
>> > cast<DependentSizedArrayType>(T);
>> >   return getDependentSizedArrayType(UnqualElt,
>> > DSAT->getSizeExpr()->Retain(),
>> >                                     DSAT->getSizeModifier(), 0,
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>