<br><br><div class="gmail_quote">2010/1/5 Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Dec 30, 2009, at 2:59 PM, Zhongxing Xu wrote:<br>
<br>
> Author: zhongxingxu<br>
> Date: Wed Dec 30 16:59:54 2009<br>
> New Revision: 92318<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=92318&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=92318&view=rev</a><br>
> Log:<br>
> The element type should also be canonicalized. Add a case for VariableArrayType.<br>
<br>
</div>Hi Zhongxing,<br>
<br>
I don't think that this patch is correct.  According to the comment (which I moved to the .cpp file):<br>
<br>
  /// \brief Returns this type as a completely-unqualified array type, capturing<br>
  /// the qualifiers in Quals. This only operates on canonical types in order<br>
  /// to ensure the ArrayType doesn't itself have qualifiers.<br>
<br>
I think it would be better to assert that the type is canonical on entry to the method.<br></blockquote><div><br>Sorry, I didn't quite understand your point. We are already asserting the QualType T is canonical on the entry to the method.<br>
<br>The patch makes sure the element type is canonical when we pass it to the method getUnqualifiedArrayType().<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

<font color="#888888"><br>
-Chris<br>
</font><div><div></div><div class="h5"><br>
><br>
> Modified:<br>
>    cfe/trunk/lib/AST/ASTContext.cpp<br>
><br>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=92318&r1=92317&r2=92318&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=92318&r1=92317&r2=92318&view=diff</a><br>

><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)<br>
> +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Dec 30 16:59:54 2009<br>
> @@ -2383,7 +2383,7 @@<br>
>   assert(!T.hasQualifiers() && "canonical array type has qualifiers!");<br>
>   const ArrayType *AT = cast<ArrayType>(T);<br>
>   QualType Elt = AT->getElementType();<br>
> -  QualType UnqualElt = getUnqualifiedArrayType(Elt, Quals);<br>
> +  QualType UnqualElt = getUnqualifiedArrayType(getCanonicalType(Elt), Quals);<br>
>   if (Elt == UnqualElt)<br>
>     return T;<br>
><br>
> @@ -2396,6 +2396,12 @@<br>
>     return getIncompleteArrayType(UnqualElt, IAT->getSizeModifier(), 0);<br>
>   }<br>
><br>
> +  if (const VariableArrayType *VAT = dyn_cast<VariableArrayType>(T)) {<br>
> +    return getVariableArrayType(UnqualElt, VAT->getSizeExpr()->Retain(),<br>
> +                                VAT->getSizeModifier(), 0,<br>
> +                                SourceRange());<br>
> +  }<br>
> +<br>
>   const DependentSizedArrayType *DSAT = cast<DependentSizedArrayType>(T);<br>
>   return getDependentSizedArrayType(UnqualElt, DSAT->getSizeExpr()->Retain(),<br>
>                                     DSAT->getSizeModifier(), 0,<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br>