[PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Ramsay Jones]

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Chris,

These warnings were issued by gcc v3.4.4, but not by gcc v4.1.2.
So I guess gcc probably found these warnings to be too noisy ...

ATB,
Ramsay Jones

 parse.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/parse.c b/parse.c
index e5ad867..42e8c74 100644
--- a/parse.c
+++ b/parse.c
@@ -2616,6 +2616,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 			case SYM_ENUM:
 			case SYM_RESTRICT:
 				base_type->ident = ident;
+			default:
+				break;
 			}
 		}
 	} else if (base_type && base_type->type == SYM_FN) {
-- 
1.6.3.1

Re: [PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Josh Triplett]

On Sat, Jul 18, 2009 at 09:41:46PM +0100, Ramsay Jones wrote:
> These warnings were issued by gcc v3.4.4, but not by gcc v4.1.2.
> So I guess gcc probably found these warnings to be too noisy ...
[...]
> --- a/parse.c
> +++ b/parse.c
> @@ -2616,6 +2616,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
>  			case SYM_ENUM:
>  			case SYM_RESTRICT:
>  				base_type->ident = ident;
> +			default:
> +				break;
>  			}

I don't think you want to add defaults like this just to avoid warnings.
Warnings like that can help when adding a new item to an enum, to find
the places where you need to extend the code to hand the new item.  And
since current GCC doesn't even issue the warning by default, it seems
even more unnecessary to add that default case.

- Josh Triplett

Re: [PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Ramsay Jones]

Josh Triplett wrote:
> On Sat, Jul 18, 2009 at 09:41:46PM +0100, Ramsay Jones wrote:
>> These warnings were issued by gcc v3.4.4, but not by gcc v4.1.2.
>> So I guess gcc probably found these warnings to be too noisy ...
> [...]
>> --- a/parse.c
>> +++ b/parse.c
>> @@ -2616,6 +2616,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
>>  			case SYM_ENUM:
>>  			case SYM_RESTRICT:
>>  				base_type->ident = ident;
>> +			default:
>> +				break;
>>  			}
> 
> I don't think you want to add defaults like this just to avoid warnings.
> Warnings like that can help when adding a new item to an enum, to find
> the places where you need to extend the code to hand the new item.  And
> since current GCC doesn't even issue the warning by default, it seems
> even more unnecessary to add that default case.
> 

OK...

So, if I understand your argument, in order to make the best use of these
warnings, then the correct change would look like the diff given below,
and (for more up-to-date gcc) add -Wswitch-enum to CFLAGS (at least
occasionally).

Yes?

[if no, then I obviously don't understand ;-)]

Hmm, I have another idea; a new patch is brewing!

ATB,
Ramsay Jones

-->8--
diff --git a/parse.c b/parse.c
index e5ad867..afe39c3 100644
--- a/parse.c
+++ b/parse.c
@@ -2616,6 +2616,23 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 			case SYM_ENUM:
 			case SYM_RESTRICT:
 				base_type->ident = ident;
+				break;
+			case SYM_UNINITIALIZED:
+			case SYM_PREPROCESSOR:
+			case SYM_BASETYPE:
+			case SYM_NODE:
+			case SYM_PTR:
+			case SYM_FN:
+			case SYM_ARRAY:
+			case SYM_TYPEDEF:
+			case SYM_TYPEOF:
+			case SYM_MEMBER:
+			case SYM_BITFIELD:
+			case SYM_LABEL:
+			case SYM_FOULED:
+			case SYM_KEYWORD:
+			case SYM_BAD:
+				break;
 			}
 		}
 	} else if (base_type && base_type->type == SYM_FN) {
-->8--

Re: [PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Christopher Li]

On Tue, Jul 21, 2009 at 2:15 PM, Ramsay Jones<ramsay@ramsay1.demon.co.uk> wrote:

>> I don't think you want to add defaults like this just to avoid warnings.
>> Warnings like that can help when adding a new item to an enum, to find
>> the places where you need to extend the code to hand the new item.  And
>> since current GCC doesn't even issue the warning by default, it seems
>> even more unnecessary to add that default case.
>>
>
> OK...
>
> So, if I understand your argument, in order to make the best use of these
> warnings, then the correct change would look like the diff given below,
> and (for more up-to-date gcc) add -Wswitch-enum to CFLAGS (at least
> occasionally).

OK. I don't want to list all the enumerate value here just for the
sake of gcc warnings.
It makes the code ugly. Nor do I want to change the gcc flags used to compile
sparse. If the newest gcc still complain on those. I would rather add
the blank default
to make it clean. Since latest gcc doesn't issue warning on those. I
think it is fine to
leave it as it is.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Josh Triplett]

On Tue, Jul 21, 2009 at 10:15:49PM +0100, Ramsay Jones wrote:
> Josh Triplett wrote:
> > On Sat, Jul 18, 2009 at 09:41:46PM +0100, Ramsay Jones wrote:
> >> These warnings were issued by gcc v3.4.4, but not by gcc v4.1.2.
> >> So I guess gcc probably found these warnings to be too noisy ...
> > [...]
> >> --- a/parse.c
> >> +++ b/parse.c
> >> @@ -2616,6 +2616,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
> >>  			case SYM_ENUM:
> >>  			case SYM_RESTRICT:
> >>  				base_type->ident = ident;
> >> +			default:
> >> +				break;
> >>  			}
> > 
> > I don't think you want to add defaults like this just to avoid warnings.
> > Warnings like that can help when adding a new item to an enum, to find
> > the places where you need to extend the code to hand the new item.  And
> > since current GCC doesn't even issue the warning by default, it seems
> > even more unnecessary to add that default case.
> > 
> 
> OK...
> 
> So, if I understand your argument, in order to make the best use of these
> warnings, then the correct change would look like the diff given below,
> and (for more up-to-date gcc) add -Wswitch-enum to CFLAGS (at least
> occasionally).
[...]
> diff --git a/parse.c b/parse.c
> index e5ad867..afe39c3 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -2616,6 +2616,23 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
>  			case SYM_ENUM:
>  			case SYM_RESTRICT:
>  				base_type->ident = ident;
> +				break;
> +			case SYM_UNINITIALIZED:
> +			case SYM_PREPROCESSOR:
> +			case SYM_BASETYPE:
> +			case SYM_NODE:
> +			case SYM_PTR:
> +			case SYM_FN:
> +			case SYM_ARRAY:
> +			case SYM_TYPEDEF:
> +			case SYM_TYPEOF:
> +			case SYM_MEMBER:
> +			case SYM_BITFIELD:
> +			case SYM_LABEL:
> +			case SYM_FOULED:
> +			case SYM_KEYWORD:
> +			case SYM_BAD:
> +				break;

This represents exactly why more recent GCC defaults this warning to
off. ;)

No, I don't think writing the code this way helps.  I think it makes
more sense to leave the warning off and not bother placating older
versions of GCC.

- Josh Triplett

Re: [PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Ramsay Jones]

Christopher Li wrote:
> OK. I don't want to list all the enumerate value here just for the
> sake of gcc warnings.
> It makes the code ugly. Nor do I want to change the gcc flags used to compile
> sparse. If the newest gcc still complain on those. I would rather add
> the blank default
> to make it clean. Since latest gcc doesn't issue warning on those. I
> think it is fine to
> leave it as it is.
> 

OK, no problem. I can maintain this in my cygwin repo, along with the
__sentinel__ attribute patch. (As before, I don't need this on Linux).

Just FYI, I had planned to fix this by implementing a local makefile
config file (like config.mak in git), so that I could fix this issue
locally ("out of tree").

However, I was (pleasantly) surprised to find that I'd been beaten to
the punch by Samuel Bronson in commit 8d86d0e.

So I tried this local.mk file:

    OS=cygwin
    CFLAGS+=-Wno-switch-enum

This lead to the invocation of gcc I was hoping for; something like:

    gcc ... -Wall ... -Wno-switch-enum parse.c

which I had expected to suppress the warnings. It didn't. *Ho-Hum* ;-)

ATB,
Ramsay Jones

Re: [PATCH 2/5] Fix some "enum value 'SYM_...' not handled in switch" warnings

[Christopher Li]

On Fri, Jul 24, 2009 at 11:25 AM, Ramsay
Jones<ramsay@ramsay1.demon.co.uk> wrote:
> Just FYI, I had planned to fix this by implementing a local makefile
> config file (like config.mak in git), so that I could fix this issue
> locally ("out of tree").
>
> However, I was (pleasantly) surprised to find that I'd been beaten to
> the punch by Samuel Bronson in commit 8d86d0e.
>
> So I tried this local.mk file:
>
>    OS=cygwin
>    CFLAGS+=-Wno-switch-enum
>
> This lead to the invocation of gcc I was hoping for; something like:
>
>    gcc ... -Wall ... -Wno-switch-enum parse.c

Yes, that works. I try the latest version of the cygwin. The gcc in
cygwin is still old enough to give the warning. I am going to leave it
as it is.

BTW, you can put some local build target in local.mk as well, not
just config changes.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html