<div class="gmail_quote">On Wed, Nov 2, 2011 at 12:36 AM, Enea Zaffanella <span dir="ltr"><<a href="mailto:zaffanella@cs.unipr.it">zaffanella@cs.unipr.it</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Il 02/11/2011 03:21, Richard Trieu ha scritto:<br>
<div class="im">> On Mon, Oct 24, 2011 at 7:30 AM, Douglas Gregor <<a href="mailto:dgregor@apple.com">dgregor@apple.com</a><br>
</div><div class="im">> <mailto:<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>>> wrote:<br>
><br>
><br>
> On Oct 20, 2011, at 5:36 PM, Chandler Carruth wrote:<br>
><br>
>> On Thu, Oct 20, 2011 at 5:12 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a><br>
</div><div><div class="h5">>> <mailto:<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>>> wrote:<br>
>><br>
>> Should Clang be printing suffixes that are accepted only with<br>
>> certain flags?<br>
>><br>
>><br>
>> I think this is an interesting policy decision. I'd love to hear<br>
>> Doug's thoughts on it.<br>
>><br>
>> It seems fine to me for Clang, when running with -fms-extensions,<br>
>> to suggest fixes even if only valid for -fms-extensions. Clearly<br>
>> if there is a generic suggestion that could be made, that would be<br>
>> a preferred alternative. For example, '__asm__' should be<br>
>> suggested before 'asm'.<br>
><br>
> I think it's fine for Clang to print suffixes that are only accepted<br>
> with certain flags. Presumably, you should never get an<br>
> IntegerLiteral of type __int128_t unless you're in a dialect that<br>
> supports parsing it.<br>
><br>
> … except that we cheat when we're building template arguments,<br>
> because it was convenient. That cheating could be eliminated by<br>
> encoding integer literal values directly within<br>
> SubstNonTypeTemplateParmExpr.<br>
><br>
> - Doug<br>
><br>
><br>
> New patch. Changes as follows:<br>
> Add int128 and uint128 suffixes (i128 and Ui128) to StmtPrinter. short<br>
> and unsigned short will get to llvm_unreachable<br>
> Add assert to IntergerLiteral to prevent creation with type short or<br>
> unsigned short<br>
> Fix comment in IntegerLiteral to say that int128 and uint128 are<br>
> acceptable types<br>
> Change BuildExpressFromIntegralTemplateArgument to give a proper Expr.<br>
> For negative numbers, use UnaryOperator of IntegerLiteral. For short<br>
> and unsigned short, ImplicitCastExpr from int.<br>
<br>
<br>
</div></div>The case of explictly signed/unsigned char template arguments should be<br>
considered too. Currently clang produces a character literal having a<br>
signed/unsigned char type in those cases, but such a character literal<br>
should typically have plain char type. Would it be possible to improve<br>
the current patch so that an integer literal is used instead (with the<br>
appropriate cast)?<br>
<br></blockquote><div>Yes, that is certainly possible to do. I will play around with it a little and see how it works out.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Regarding the kind of cast: I am fully aware that here it is mainly a<br>
matter of taste ... but wouldn't it be slightly better to use explicit<br>
casts, so that the required conversions are more clearly readable from<br>
the pretty printed statement?<br>
<br>
For instance, consider the following:<br>
<br>
template <short v> struct S {<br>
static const long a = v;<br>
};<br>
<br>
S<0> s;<br>
<br>
Here, when initializing the static member `a', the short value `v' is<br>
implicitly converted to long. If we go for implicit casts, we will<br>
obtain a template instance where the 0 integer literal is first<br>
implicitly converted to short and then implicitly converted to long,<br>
which is somehow strange looking. An explicit cast from the integer<br>
literal to short may serve as a better hint for the reader regarding the<br>
fact that such a 0 constant value was originally having type short.<br></blockquote><div><br></div><div>This is what the AST dump looks like:</div><div><br></div><div><div>template <short v = 0> struct S {</div><div>
struct S;</div><div> static const long a = (ImplicitCastExpr 0x4664d48 <unsignedtemplate.cc:24:26> 'const long' <IntegralCast></div><div> (SubstNonTypeTemplateParmExpr 0x4664d20 <col:26> 'short'</div>
<div> (ImplicitCastExpr 0x4664d08 <col:26> 'short' <IntegralCast></div><div> (IntegerLiteral 0x4664ce0 <col:26> 'int' 0))))</div><div>;</div><div> inline S() throw() (CompoundStmt 0x4664ff8 <unsignedtemplate.cc:23:28>)</div>
<div><br></div><div><br></div><div> inline S(const S<0> &) throw();</div><div>}</div></div><div><br></div><div>The two implicit casts are separated by a SubstNonTypeTemplateParamExpr. It shows the integer literal cast to short to match the non-type template parameter before being case to long for assignment.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Cheers,<br>
Enea.<br>
<div class="im"><br>
<br>
> Basically, protect IntegerLiteral being shorts, fix the only case of<br>
> short IntegerLiterals, and add support for printing int128<br>
> IntegerLiterals.<br>
><br>
> PR:<br>
> <a href="http://llvm.org/bugs/show_bug.cgi?id=11179" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=11179</a><br>
><br>
> Patch also located at:<br>
> <a href="http://codereview.appspot.com/5309045/" target="_blank">http://codereview.appspot.com/5309045/</a><br>
><br>
><br>
><br>
</div>> _______________________________________________<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>
</blockquote></div><br>