On Fri, Jul 22, 2016 at 12:44:30PM +0200, Marek Polacek wrote: > ...and the boring part. It found a few bugs, e.g. in jcf-dump.c > and rs6000.c - I fixed those. > > I think generally it's better to use the attribute rather than a falls > through comment, because the latter can't be followed by other comment > or a macro to work.
The comment has the advantage (at least if it is some well standardized one) that lint/coverity and other tools already understand it. So I think using the comment in the cases where it really is immediately before case/default is better. > @@ -32335,6 +32341,7 @@ rs6000_handle_altivec_attribute (tree *node, > case V4SImode: case V8HImode: case V16QImode: case V4SFmode: > case V2DImode: case V2DFmode: > result = type; > + gcc_fallthrough (); > default: break; > } > break; > @@ -32345,6 +32352,7 @@ rs6000_handle_altivec_attribute (tree *node, > case SImode: case V4SImode: result = bool_V4SI_type_node; break; > case HImode: case V8HImode: result = bool_V8HI_type_node; break; > case QImode: case V16QImode: result = bool_V16QI_type_node; > + gcc_fallthrough (); > default: break; > } > break; > @@ -32352,6 +32360,7 @@ rs6000_handle_altivec_attribute (tree *node, > switch (mode) > { > case V8HImode: result = pixel_V8HI_type_node; > + gcc_fallthrough (); > default: break; > } > default: break; I thought we don't warn on these anymore? Also, as others said, I think it would be best to split this patch into: 1) bugfixes part (where you've added break; that was missing and it changes (fixes) behavior) 2) questionable cases (your XX), with request for the corresponding maintainers to decide 3) the actual addition of the attribute/comments or tweaking their wording, so that for intentional fallthrus we don't warn Jakub