On Mon, Jan 4, 2010 at 10:36 PM, Zhongxing Xu <[email protected]> wrote: > > > 2010/1/5 Chris Lattner <[email protected]> >> >> 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 >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
