[patch] Improve diagnostic on default-initializing const variables (PR20208)

[Nico Weber]

On Tue, Jul 22, 2014 at 7:35 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> +/// Prints a fixit for adding a null initializer for |Entity|. Call this
> only
> +/// right after emitting a diagnostic.
> +static void maybeEmitNullInitializationFixit(Sema &S,
> +                                             InitializationSequence
> &Sequence,
> +                                             const InitializedEntity
> &Entity) {
>
> s/null/zero/g
>
> Otherwise, LGTM.
>

r213725, thanks!


> On Tue, Jul 22, 2014 at 9:18 AM, Nico Weber <thakis at chromium.org> wrote:
>
>>  Ideas? Easiest would be to put the fixit on a note, but the note would
>>>> just repeat the snippet from the diag itself, which isn't great.
>>>>
>>>
>>> We do that in a lot of cases; this wouldn't be so bad, and it'd be
>>> safer. The current approach is also workable, because we never make a
>>> choice based on whether default initialization of a variable is valid, but
>>> it seems fragile.
>>>
>>> The nice thing about putting it in a separate note is that you can point
>>> the caret directly at the place where the fixit text should be inserted,
>>> while still having the main diagnostic point at the entity being
>>> initialized; that tends to make fixits much more obvious.
>>>
>>
>> Ok, here's a patch that adds the fixit on a note instead. The code change
>> is now very simple. The diagnostic looks like this:
>>
>> test.cc:2:9: error: default initialization of an object of const type
>> 'const A' without a user-provided default constructor
>> const A aasdf;
>>         ^
>> test.cc:2:14: note: add an explicit initializer to initialize 'aasdf'
>> const A aasdf;
>>              ^
>>               = {}
>> 1 error generated.
>>
>> Maybe it should be " add an explicit initializer for 'aasdf'"? That's
>> shorter, but it sounds too confident maybe.
>>
>> The test case updates are mostly boring.
>> In cxx1y-variable-templates_in_class.cpp, the variable name for a template
>> is printed as "…to initialize 'b<int, int>'" with a note "in instantiation
>> of". I found that a bit surprising, but it matches the diagnostic on the
>> same line, so I suppose that's ok.
>>
>
> You could use %q0 in the diagnostic if you want to see 'B4::b<int, int>'
> here. (If it's not that, I'm not sure which part you find surprising here.)
>

Seeing concrete types on a template line – if I add an initializer, it'll
be used for all (non-specialized) instantiations.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140722/334d1a62/attachment.html>