On Wed, 2016-08-31 at 11:09 +0200, Marek Polacek wrote:
> On Tue, Aug 30, 2016 at 11:22:21AM -0400, David Malcolm wrote:
> > On Tue, 2016-08-30 at 16:50 +0200, Marek Polacek wrote:
> > > On Tue, Aug 30, 2016 at 04:40:57PM +0200, Bernd Schmidt wrote:
> > > > On 08/30/2016 04:38 PM, David Malcolm wrote:
> > > >
> > > > > In conjunction with the not-yet-in-trunk -fdiagnostics
> > > > > -generate
> > > > > -patch,
> > > > > this can generate patches like this:
> > > > >
> > > > > --- modernize-named-inits.c
> > > > > +++ modernize-named-inits.c
> > > > > @@ -16,7 +16,7 @@
> > > > > /* Old-style named initializers. */
> > > > >
> > > > > struct foo old_style_f = {
> > > > > - foo: 1,
> > > > > - bar: 2,
> > > > > + .foo= 1,
> > > > > + .bar= 2,
> > > > > };
> > > >
> > > > What happens if there are newlines in between any of the
> > > > tokens?
> > >
> > > It's easy to check for yourself: when the identifier and colon
> > > are
> > > not
> > > on the same line, we print something like
> > >
> > > w.c: In function ‘main’:
> > > w.c:7:1: warning: obsolete use of designated initializer with ‘:’
> > > [
> > > -Wpedantic]
> > > :
> > > ^
> > > =
> > >
> > > which I don't think is desirable -- giving up on the fix-it hint
> > > in
> > > such case
> > > could be appropriate.
> >
> > I think that's a bug in diagnostic-show-locus.c; currently it only
> > prints lines and fixits for the lines references in the ranges
> > within
> > the rich_location. I'll try to fix that.
> >
> > > I admit I dislike the lack of a space before = in ".bar= 2", but
> > > using
> > >
> > > richloc.add_fixit_replace (colon_loc, " =");
> > >
> > > wouldn't work for "foo : 1" I think.
> >
> > I actually tried that first, but I didn't like the way it was
> > printed
> > e.g.:
> >
> > w.c: In function ‘main’:> w.c:7:1: warning: obsolete use of
> > designated
> > initializer with ‘:’ [-Wpedantic]>
> > foo: 1,
> > ^
> > . =
> >
> > I'm looking at rewriting how fixits get printed, to print the
> > affected
> > range of the affected lines (using the edit-context.c work posted
> > last
> > week), so this would appear as:
> >
> > foo: 1,
> > ^
> > .foo =
>
> This would be perfect.
BTW, this is currently blocked, I need review of this patch:
"[PATCH 2/4] Improvements to typed_splay_tree"
* https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01777.html
(I've rewritten edit_context to work on a per-line basis, so it should
now be efficient enough for this use-case)
> > Also, there may be a case for adding some smarts to
> > gcc_rich_location for adding fixits in a formatting-aware way, by
> > looking at the neighboring whitespace (this might help for the
> > issue with adding "break;" etc in the fallthru patch kit).
>
> Thanks. I hope it won't be too hard to implement :/
I'll try...
