# [PATCH] sequencer: shut up clang warning 3 messages from 2016-11-09 to 2016-11-09. Participants: Johannes Schindelin, Jeff King, Jakub Narębski. Thread: https://gitlist.dev/t/44447 ## Johannes Schindelin, 2016-11-09 13:56 Subject: [PATCH] sequencer: shut up clang warning Message-ID: URL: https://gitlist.dev/e/efbba4b32515fed7096c1c81dbe97eedd44083b0.1478699713.git.johannes.schindelin%40gmx.de ``` 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 --- 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 ``` ## Jeff King, 2016-11-09 15:37 Subject: Re: [PATCH] sequencer: shut up clang warning Message-ID: <20161109153733.iauja57zikyb2awv@sigill.intra.peff.net> URL: https://gitlist.dev/e/20161109153733.iauja57zikyb2awv%40sigill.intra.peff.net In-Reply-To: ``` 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 ``` ## Jakub Narębski, 2016-11-09 22:28 Subject: Re: [PATCH] sequencer: shut up clang warning Message-ID: <11e953de-956d-d4eb-8117-ebbfc410676b@gmail.com> URL: https://gitlist.dev/e/11e953de-956d-d4eb-8117-ebbfc410676b%40gmail.com In-Reply-To: ``` 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 > --- > 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 ```