[PATCH] sequencer: shut up clang warning

[PATCH] sequencer: shut up clang warning

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]

When comparing a value of type `enum todo_command` with a value that is
outside the defined enum constants, clang greets the developer with this
warning:

	comparison of constant 2 with expression of type
	'const enum todo_command' is always true

While this is arguably true *iff* the value was never cast from a
free-form int, we should keep the cautious code in place.

To shut up clang, we simply introduce an otherwise pointless enum constant
and compare against that.

Noticed by Torsten Bögershausen.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/enum-warning-v1
Fetch-It-Via: git fetch https://github.com/dscho/git enum-warning-v1

	Peff, if you would like me to include more of your commit message,
	please let me know.

	I will adjust the sequencer-i patches to keep the TODO_INVALID
	constant.

 sequencer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5fd75f3..f80e9c0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
 
 enum todo_command {
 	TODO_PICK = 0,
-	TODO_REVERT
+	TODO_REVERT,
+	TODO_INVALID
 };
 
 static const char *todo_command_strings[] = {
@@ -629,7 +630,7 @@ static const char *todo_command_strings[] = {
 
 static const char *command_to_string(const enum todo_command command)
 {
-	if (command < ARRAY_SIZE(todo_command_strings))
+	if (command < TODO_INVALID)
 		return todo_command_strings[command];
 	die("Unknown command: %d", command);
 }

base-commit: be5a750939c212bc0781ffa04fabcfd2b2bd744e
-- 
2.10.1.583.g721a9e0

Re: [PATCH] sequencer: shut up clang warning

[Jeff King]

On Wed, Nov 09, 2016 at 02:56:25PM +0100, Johannes Schindelin wrote:

> When comparing a value of type `enum todo_command` with a value that is
> outside the defined enum constants, clang greets the developer with this
> warning:
> 
> 	comparison of constant 2 with expression of type
> 	'const enum todo_command' is always true
> 
> While this is arguably true *iff* the value was never cast from a
> free-form int, we should keep the cautious code in place.
> 
> To shut up clang, we simply introduce an otherwise pointless enum constant
> and compare against that.

This does silence the warning.

I slightly prefer mine because:

  1. It does not carry an implicit requirement that TODO_INVALID remain
     the final enum value.

  2. It also protects the range check against a negative enum value.

But this is code that is getting changed later by you anyway, and I do
not care that strongly.

-Peff

Re: [PATCH] sequencer: shut up clang warning

[Jakub Narębski]

W dniu 09.11.2016 o 14:56, Johannes Schindelin pisze:

> When comparing a value of type `enum todo_command` with a value that is
> outside the defined enum constants, clang greets the developer with this
> warning:
> 
> 	comparison of constant 2 with expression of type
> 	'const enum todo_command' is always true
> 
> While this is arguably true *iff* the value was never cast from a
> free-form int, we should keep the cautious code in place.
> 
> To shut up clang, we simply introduce an otherwise pointless enum constant
> and compare against that.
> 
> Noticed by Torsten Bögershausen.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

>  sequencer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 5fd75f3..f80e9c0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
>  
>  enum todo_command {
>  	TODO_PICK = 0,
> -	TODO_REVERT
> +	TODO_REVERT,
> +	TODO_INVALID
>  };

Why not name it TODO_N, or N_TODO, or something like that?

-- 
Jakub Narębski