<div dir="ltr"><div>LGTM with tiny tweaks:</div><div><br></div><div>+def err_ice_too_large : Error<</div><div>+  "integer constant evaluates to value %0 that cannot be represented in a "</div><div>+  "%1-bit %select{signed|unsigned}2 integer type">;</div>
<div><br></div><div>"integer constant expression evaluates to ..." would better match standard terminology.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra">
+def ext_integer_literal_too_large_for_signed : ExtWarn<</div><div class="gmail_extra">+  "integer literal is too large to be represented in a signed integer type, "</div><div class="gmail_extra">+  "and will instead be interpreted as unsigned integer type">,</div>
<div><br></div><div>You could drop the "integer type" in the last line, or maybe simply say ", treating as unsigned" or ", interpreted as unsigned". [This is going to look weird with -pedantic-errors (where this diagnostic is an error) but I can't think of a way of wording this which works in that case.]</div>
</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 22, 2014 at 6:04 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Latest incarnation.<br>
<span class=""><font color="#888888"><br>
~Aaron<br>
</font></span><div class=""><div class="h5"><br>
On Tue, Jul 22, 2014 at 8:50 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
> On Tue, Jul 22, 2014 at 8:45 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>> On Tue, Jul 22, 2014 at 5:38 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>> wrote:<br>
>>><br>
>>> How do these sound to you?<br>
>>><br>
>>> On Tue, Jul 22, 2014 at 8:13 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>>> wrote:<br>
>>> > +def err_integer_literal_too_large : Error<<br>
>>> > +  "integer literal is a value that cannot be represented as<br>
>>> > %select{a|an}0<br>
>>> > "<br>
>>> > +  "%select{signed|unsigned}0 integer">;<br>
>>> ><br>
>>> > I don't think the "is a value that" adds anything here. Also, I'd like<br>
>>> > to<br>
>>> > see something that more directly says we don't have a signed/unsigned<br>
>>> > integer type large enough.<br>
>>><br>
>>> "integer literal is too large to be represented in %select{a|an}0<br>
>>> %select{signed|unsigned}0 integer type"<br>
>><br>
>><br>
>> Perhaps "... to be represented in any %select{signed |}0integer type"?<br>
><br>
> Works for me.<br>
><br>
>><br>
>>><br>
>>> >  def err_integer_too_large : Error<<br>
>>> > -  "integer constant is larger than the largest %0-bit unsigned integer<br>
>>> > type">;<br>
>>> > -def ext_integer_too_large_for_signed : ExtWarn<<br>
>>> > -  "integer constant is larger than the largest %0-bit signed integer<br>
>>> > type">,<br>
>>> > +  "integer constant evaluates to value %0 that cannot be represented as<br>
>>> > a "<br>
>>> > +  "%1-bit %select{signed|unsigned}2 integer">;<br>
>>> ><br>
>>> > Please rename this to something about ICEs, and move it to<br>
>>> > DiagnosticSemaKinds next to the existing err_ice_ diagnostics. (This is<br>
>>> > pretty similar to ext_cce_narrowing...)<br>
>>> ><br>
>>> ><br>
>>> > +def ext_integer_literal_too_large_for_signed : ExtWarn<<br>
>>> > +  "integer literal is a value that cannot be represented as a signed<br>
>>> > integer, "<br>
>>> > +  "and will instead be interpreted as unsigned">,<br>
>>> ><br>
>>> > Again, mentioning integer types and not just integers would make this<br>
>>> > clearer.<br>
>>><br>
>>> "integer literal is too large to be represented in a signed integer<br>
>>> type, and will instead be interpreted as unsigned integer type"<br>
>><br>
>><br>
>> Can we specify the type here? I think it's always 'unsigned long long' when<br>
>> we reach this diagnostic.<br>
><br>
> It's not always unsigned long long -- this gets used in EvaluateValue<br>
> in PPExpressions.cpp, and we only set the resulting value to unsigned,<br>
> but not change its type.<br></div></div></blockquote><div><br></div><div>Setting it to unsigned is effectively changing its type from intmax_t to uintmax_t (we don't model types in the preprocessor, so there's nothing else to update here). Ideally I'd still like for us to talk about a type in this diagnostic but your patch is fine to commit without that.</div>
</div></div></div>