[PATCH] sequencer: silence -Wtautological-constant-out-of-range-compare

Jeff King· Nov 9, 2016, 03:57 UTC · lore

When clang compiles sequencer.c, it complains:

  sequencer.c:632:14: warning: comparison of constant 2 with
    expression of type 'const enum todo_command' is always
    true [-Wtautological-constant-out-of-range-compare]
          if (command < ARRAY_SIZE(todo_command_strings))

This is because "command" is an enum that may only have two values (0 and 1) and the array in question has two elements.

As it turns out, clang is actually wrong here, at least according to its own bug tracker:

  https://llvm.org/bugs/show_bug.cgi?id=16154

But it's still worth working around this, as the warning is present with -Wall, meaning we fail compilation with "make DEVELOPER=1".

Casting the enum to size_t sufficiently unconfuses clang. As a bonus, it also catches any possible out-of-bounds access if the enum takes on a negative value (which shouldn't happen either, but again, this is a defensive check).

Signed-off-by: Jeff King <peff@peff.net>
---
I know that a different fix is coming in a follow-on series, but I think
it's worth doing this to un-break clang on master (and v2.11) in the
meantime.

 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to sequencer.c +1 −1

diff --git a/sequencer.c b/sequencer.c
index 5fd75f30d..6f0ff9e41 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -629,7 +629,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 ((size_t)command < ARRAY_SIZE(todo_command_strings))
 		return todo_command_strings[command];
 	die("Unknown command: %d", command);
 }

-- 
2.11.0.rc0.263.g6f44bc3

Johannes Schindelin· Nov 9, 2016, 13:54 UTC · re: Jeff King · lore

Re: [PATCH] sequencer: silence -Wtautological-constant-out-of-range-compare

Hi Peff,

On Tue, 8 Nov 2016, Jeff King wrote:

Show 12 quoted lines

> diff --git a/sequencer.c b/sequencer.c
> index 5fd75f30d..6f0ff9e41 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -629,7 +629,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 ((size_t)command < ARRAY_SIZE(todo_command_strings))
>  		return todo_command_strings[command];
>  	die("Unknown command: %d", command);

I have come to prefer a slightly different approach. Will send it out in a moment.

Ciao, Dscho

patchsequencer: silence -Wtautological-constant-out-of-range-compare