* [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings.
@ 2016-11-02 13:19 Mark Wielaard
2016-11-02 13:24 ` Jakub Jelinek
2016-11-18 23:45 ` Allan Sandfeld Jensen
0 siblings, 2 replies; 4+ messages in thread
From: Mark Wielaard @ 2016-11-02 13:19 UTC (permalink / raw)
To: gcc-patches; +Cc: Mark Wielaard
Adjust some comments, add some explicit fall through comments or explicit
returns where necessary to not get implicit-fallthrough warnings.
All fall throughs were deliberate. In one case I added an explicit return
false for clarity instead of falling through a default case (that also
would return false).
libiberty/ChangeLog:
* cplus-dem.c (demangle_signature): Move fall through comment.
(demangle_fund_type): Add fall through comment between 'G' and 'I'.
* hashtab.c (iterative_hash): Add fall through comments.
* regex.c (regex_compile): Add Fall through comment after '+'/'?'.
(byte_re_match_2_internal): Add Fall through comment after jump_n.
Change "Note fall through" to "Fall through".
(common_op_match_null_string_p): Return false after set_number_at
instead of fall through.
---
diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index 7f63397..810e971 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -1658,8 +1658,8 @@ demangle_signature (struct work_stuff *work,
break;
}
else
- /* fall through */
{;}
+ /* fall through */
default:
if (AUTO_DEMANGLING || GNU_DEMANGLING)
@@ -4024,6 +4024,7 @@ demangle_fund_type (struct work_stuff *work,
success = 0;
break;
}
+ /* fall through */
case 'I':
(*mangled)++;
if (**mangled == '_')
diff --git a/libiberty/hashtab.c b/libiberty/hashtab.c
index 04607ea..99381b1 100644
--- a/libiberty/hashtab.c
+++ b/libiberty/hashtab.c
@@ -962,17 +962,17 @@ iterative_hash (const PTR k_in /* the key */,
c += length;
switch(len) /* all the case statements fall through */
{
- case 11: c+=((hashval_t)k[10]<<24);
- case 10: c+=((hashval_t)k[9]<<16);
- case 9 : c+=((hashval_t)k[8]<<8);
+ case 11: c+=((hashval_t)k[10]<<24); /* fall through */
+ case 10: c+=((hashval_t)k[9]<<16); /* fall through */
+ case 9 : c+=((hashval_t)k[8]<<8); /* fall through */
/* the first byte of c is reserved for the length */
- case 8 : b+=((hashval_t)k[7]<<24);
- case 7 : b+=((hashval_t)k[6]<<16);
- case 6 : b+=((hashval_t)k[5]<<8);
- case 5 : b+=k[4];
- case 4 : a+=((hashval_t)k[3]<<24);
- case 3 : a+=((hashval_t)k[2]<<16);
- case 2 : a+=((hashval_t)k[1]<<8);
+ case 8 : b+=((hashval_t)k[7]<<24); /* fall through */
+ case 7 : b+=((hashval_t)k[6]<<16); /* fall through */
+ case 6 : b+=((hashval_t)k[5]<<8); /* fall through */
+ case 5 : b+=k[4]; /* fall through */
+ case 4 : a+=((hashval_t)k[3]<<24); /* fall through */
+ case 3 : a+=((hashval_t)k[2]<<16); /* fall through */
+ case 2 : a+=((hashval_t)k[1]<<8); /* fall through */
case 1 : a+=k[0];
/* case 0: nothing left to add */
}
diff --git a/libiberty/regex.c b/libiberty/regex.c
index 9ffc3f4..6854e3b 100644
--- a/libiberty/regex.c
+++ b/libiberty/regex.c
@@ -2493,6 +2493,7 @@ PREFIX(regex_compile) (const char *ARG_PREFIX(pattern),
if ((syntax & RE_BK_PLUS_QM)
|| (syntax & RE_LIMITED_OPS))
goto normal_char;
+ /* Fall through. */
handle_plus:
case '*':
/* If there is no previous pattern... */
@@ -6697,6 +6698,7 @@ byte_re_match_2_internal (struct re_pattern_buffer *bufp,
{
case jump_n:
is_a_jump_n = true;
+ /* Fall through. */
case pop_failure_jump:
case maybe_pop_jump:
case jump:
@@ -7125,7 +7127,7 @@ byte_re_match_2_internal (struct re_pattern_buffer *bufp,
DEBUG_PRINT1 (" Match => jump.\n");
goto unconditional_jump;
}
- /* Note fall through. */
+ /* Fall through. */
/* The end of a simple repeat has a pop_failure_jump back to
@@ -7150,7 +7152,7 @@ byte_re_match_2_internal (struct re_pattern_buffer *bufp,
dummy_low_reg, dummy_high_reg,
reg_dummy, reg_dummy, reg_info_dummy);
}
- /* Note fall through. */
+ /* Fall through. */
unconditional_jump:
#ifdef _LIBC
@@ -7453,6 +7455,7 @@ byte_re_match_2_internal (struct re_pattern_buffer *bufp,
{
case jump_n:
is_a_jump_n = true;
+ /* Fall through. */
case maybe_pop_jump:
case pop_failure_jump:
case jump:
@@ -7718,6 +7721,7 @@ PREFIX(common_op_match_null_string_p) (UCHAR_T **p, UCHAR_T *end,
case set_number_at:
p1 += 2 * OFFSET_ADDRESS_SIZE;
+ return false;
default:
/* All other opcodes mean we cannot match the empty string. */
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings.
2016-11-02 13:19 [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings Mark Wielaard
@ 2016-11-02 13:24 ` Jakub Jelinek
2016-11-04 23:55 ` Mark Wielaard
2016-11-18 23:45 ` Allan Sandfeld Jensen
1 sibling, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2016-11-02 13:24 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gcc-patches
On Wed, Nov 02, 2016 at 02:19:33PM +0100, Mark Wielaard wrote:
> Adjust some comments, add some explicit fall through comments or explicit
> returns where necessary to not get implicit-fallthrough warnings.
>
> All fall throughs were deliberate. In one case I added an explicit return
> false for clarity instead of falling through a default case (that also
> would return false).
>
> libiberty/ChangeLog:
>
> * cplus-dem.c (demangle_signature): Move fall through comment.
> (demangle_fund_type): Add fall through comment between 'G' and 'I'.
> * hashtab.c (iterative_hash): Add fall through comments.
> * regex.c (regex_compile): Add Fall through comment after '+'/'?'.
> (byte_re_match_2_internal): Add Fall through comment after jump_n.
> Change "Note fall through" to "Fall through".
> (common_op_match_null_string_p): Return false after set_number_at
> instead of fall through.
LGTM, except for:
> --- a/libiberty/cplus-dem.c
> +++ b/libiberty/cplus-dem.c
> @@ -1658,8 +1658,8 @@ demangle_signature (struct work_stuff *work,
> break;
> }
> else
> - /* fall through */
> {;}
> + /* fall through */
I think you should just remove the else and {;} and just have fallthrough
comment indented where else used to be.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings.
2016-11-02 13:24 ` Jakub Jelinek
@ 2016-11-04 23:55 ` Mark Wielaard
0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2016-11-04 23:55 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Wed, 2016-11-02 at 14:24 +0100, Jakub Jelinek wrote:
> On Wed, Nov 02, 2016 at 02:19:33PM +0100, Mark Wielaard wrote:
> > libiberty/ChangeLog:
> >
> > * cplus-dem.c (demangle_signature): Move fall through comment.
> > (demangle_fund_type): Add fall through comment between 'G' and 'I'.
> > * hashtab.c (iterative_hash): Add fall through comments.
> > * regex.c (regex_compile): Add Fall through comment after '+'/'?'.
> > (byte_re_match_2_internal): Add Fall through comment after jump_n.
> > Change "Note fall through" to "Fall through".
> > (common_op_match_null_string_p): Return false after set_number_at
> > instead of fall through.
>
> LGTM, except for:
>
> > --- a/libiberty/cplus-dem.c
> > +++ b/libiberty/cplus-dem.c
> > @@ -1658,8 +1658,8 @@ demangle_signature (struct work_stuff *work,
> > break;
> > }
> > else
> > - /* fall through */
> > {;}
> > + /* fall through */
>
> I think you should just remove the else and {;} and just have fallthrough
> comment indented where else used to be.
Thanks. Pushed with that change.
Cheers,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings.
2016-11-02 13:19 [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings Mark Wielaard
2016-11-02 13:24 ` Jakub Jelinek
@ 2016-11-18 23:45 ` Allan Sandfeld Jensen
1 sibling, 0 replies; 4+ messages in thread
From: Allan Sandfeld Jensen @ 2016-11-18 23:45 UTC (permalink / raw)
To: gcc-patches
On Wednesday 02 November 2016, Mark Wielaard wrote:
> - case 11: c+=((hashval_t)k[10]<<24);
> - case 10: c+=((hashval_t)k[9]<<16);
> - case 9 : c+=((hashval_t)k[8]<<8);
> + case 11: c+=((hashval_t)k[10]<<24); /* fall through */
> + case 10: c+=((hashval_t)k[9]<<16); /* fall through */
> + case 9 : c+=((hashval_t)k[8]<<8); /* fall through */
> /* the first byte of c is reserved for the length */
This really highlights another exception -Wimplicit-fallthough should tolerate
at least on level 1. Single line of code.
case X: my_statement();
case y: my_statement();
and
case X:
my_statement();
case y:
my_statement();
In both cases, the lack of break is obvious at a glance and thus a comment
highlighting it has never been needed and shouldn't be enforced.
Well, at least in my opinion, and it would take away all the rest of false
positives I run into.
Best regards
`Allan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-18 23:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-02 13:19 [PATCH] libiberty: Fix -Wimplicit-fallthrough warnings Mark Wielaard
2016-11-02 13:24 ` Jakub Jelinek
2016-11-04 23:55 ` Mark Wielaard
2016-11-18 23:45 ` Allan Sandfeld Jensen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).