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 <[email protected]> wrote: > On Tue, Jul 22, 2014 at 7:35 PM, Richard Smith <[email protected]> > 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 <[email protected]> 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. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
