Re: [PING] Re: [PATCH] Fix-it hints for -Wimplicit-fallthrough

[Marek Polacek <polacek at redhat dot com>]

On Fri, May 26, 2017 at 02:13:56PM -0600, Martin Sebor wrote:
> On 05/26/2017 01:59 PM, David Malcolm wrote:
> > Ping:
> >   https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00334.html
> > 
> > On Thu, 2017-05-04 at 14:16 -0400, David Malcolm wrote:
> > > As of r247522, fix-it-hints can suggest the insertion of new lines.
> > > 
> > > This patch updates -Wimplicit-fallthrough to provide suggestions
> > > with fix-it hints, showing the user where to insert "break;" or
> > > fallthrough attributes.
> > > 
> > > For example:
> > > 
> > >  test.c: In function 'set_x':
> > >  test.c:15:9: warning: this statement may fall through [-Wimplicit
> > > -fallthrough=]
> > >         x = a;
> > >         ~~^~~
> > >  test.c:22:5: note: here
> > >       case 'b':
> > >       ^~~~
> > >  test.c:22:5: note: insert '__attribute__ ((fallthrough));' to
> > > silence this warning
> > >  +        __attribute__ ((fallthrough));
> > >       case 'b':
> > >       ^~~~
> > >  test.c:22:5: note: insert 'break;' to avoid fall-through
> > >  +        break;
> > >       case 'b':
> > >       ^~~~
> 
> I haven't read the patch but the notes above make me wonder:
> 
> If the location of at least one of t hints is always the same
> as that of the first "note: here" would it make sense to lose
> the latter and reduce the size of the output?  (Or lose it in
> the cases where one of the fix-it hints does share a location
> with it).

I agree that it's a tad verbose but I'm not sure if it'd be easy
to suppress printing
     case 2:
     ^~~~
more times simple enough.  So I'd be fine with the current state.
I'd also be happy with e.g.

a.c: In function ‘foo’:
a.c:7:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
       x = 1;
       ~~^~~
a.c:8:5: note: here
     case 2:
     ^~~~
a.c:8:5: note: insert ‘__attribute__ ((fallthrough));’ to silence this warning
+        __attribute__ ((fallthrough));
	 or insert ‘break;’ to avoid fall-through
+        break;

but I don't think we've got the means to do so.

On the patch, I'd think that add_newline_fixit_with_indentation doesn't
belong to gimplify.c, even though it only has one user.  But I won't be
able to approve it in any case.

	Marek