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

[Nico Weber]

Since it turns out we need to recover from this in microsoft mode, I
reverted r213725 and landed the original patch (plus a test for the fixit,
plus weakening this error to a warning on selectany decls in microsoft
mode) in r235166.

On Tue, Jul 22, 2014 at 10:25 PM, Nico Weber <thakis at chromium.org> wrote:

> 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/20150417/29fe5c32/attachment.html>