r213657 - Provide extra information in the "integer constant is too large" diagnostic. This will be used to improve other diagnostics.

[Aaron Ballman]

Thank you for all the feedback -- I've made these changes and
committed in r213865.

~Aaron

On Tue, Jul 22, 2014 at 10:16 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> LGTM with tiny tweaks:
>
> +def err_ice_too_large : Error<
> +  "integer constant evaluates to value %0 that cannot be represented in a "
> +  "%1-bit %select{signed|unsigned}2 integer type">;
>
> "integer constant expression evaluates to ..." would better match standard
> terminology.
>
>
> +def ext_integer_literal_too_large_for_signed : ExtWarn<
> +  "integer literal is too large to be represented in a signed integer type,
> "
> +  "and will instead be interpreted as unsigned integer type">,
>
> 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.]
>
> On Tue, Jul 22, 2014 at 6:04 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> Latest incarnation.
>>
>> ~Aaron
>>
>> On Tue, Jul 22, 2014 at 8:50 PM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>> > On Tue, Jul 22, 2014 at 8:45 PM, Richard Smith <richard at metafoo.co.uk>
>> > wrote:
>> >> On Tue, Jul 22, 2014 at 5:38 PM, Aaron Ballman <aaron at aaronballman.com>
>> >> wrote:
>> >>>
>> >>> How do these sound to you?
>> >>>
>> >>> On Tue, Jul 22, 2014 at 8:13 PM, Richard Smith <richard at metafoo.co.uk>
>> >>> wrote:
>> >>> > +def err_integer_literal_too_large : Error<
>> >>> > +  "integer literal is a value that cannot be represented as
>> >>> > %select{a|an}0
>> >>> > "
>> >>> > +  "%select{signed|unsigned}0 integer">;
>> >>> >
>> >>> > I don't think the "is a value that" adds anything here. Also, I'd
>> >>> > like
>> >>> > to
>> >>> > see something that more directly says we don't have a
>> >>> > signed/unsigned
>> >>> > integer type large enough.
>> >>>
>> >>> "integer literal is too large to be represented in %select{a|an}0
>> >>> %select{signed|unsigned}0 integer type"
>> >>
>> >>
>> >> Perhaps "... to be represented in any %select{signed |}0integer type"?
>> >
>> > Works for me.
>> >
>> >>
>> >>>
>> >>> >  def err_integer_too_large : Error<
>> >>> > -  "integer constant is larger than the largest %0-bit unsigned
>> >>> > integer
>> >>> > type">;
>> >>> > -def ext_integer_too_large_for_signed : ExtWarn<
>> >>> > -  "integer constant is larger than the largest %0-bit signed
>> >>> > integer
>> >>> > type">,
>> >>> > +  "integer constant evaluates to value %0 that cannot be
>> >>> > represented as
>> >>> > a "
>> >>> > +  "%1-bit %select{signed|unsigned}2 integer">;
>> >>> >
>> >>> > Please rename this to something about ICEs, and move it to
>> >>> > DiagnosticSemaKinds next to the existing err_ice_ diagnostics. (This
>> >>> > is
>> >>> > pretty similar to ext_cce_narrowing...)
>> >>> >
>> >>> >
>> >>> > +def ext_integer_literal_too_large_for_signed : ExtWarn<
>> >>> > +  "integer literal is a value that cannot be represented as a
>> >>> > signed
>> >>> > integer, "
>> >>> > +  "and will instead be interpreted as unsigned">,
>> >>> >
>> >>> > Again, mentioning integer types and not just integers would make
>> >>> > this
>> >>> > clearer.
>> >>>
>> >>> "integer literal is too large to be represented in a signed integer
>> >>> type, and will instead be interpreted as unsigned integer type"
>> >>
>> >>
>> >> Can we specify the type here? I think it's always 'unsigned long long'
>> >> when
>> >> we reach this diagnostic.
>> >
>> > It's not always unsigned long long -- this gets used in EvaluateValue
>> > in PPExpressions.cpp, and we only set the resulting value to unsigned,
>> > but not change its type.
>
>
> 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.