Subject: Re: [PATCH 04/10] parse-options.c: use exhaustive "case" arms for "enum parse_opt_type"

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Sep 2021 17:20:21 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Change code in get_value(), parse_options_check() etc. to do away with
> the "default" case in favor of exhaustively checking the relevant
> fields.

I am not sure if this is a good idea in the bigger picture.

If we know that unlisted cases should not happen, having "default:
BUG()" without explicitly listing them is just as expressive as the
result of this patch with much shorter code.

When a new enum member is added without adding corresponding
processing of the new value, either way it will be caught as a
BUG().  Removing "default: BUG()" does allow you to catch such an
error at compilation time, and keeping it may prevent you from doing
so, but in practice, you'd be adding test coverage for the new case,
which means that, even if your compiler is not cooperating, your
test suite addition will hit "default: BUG();" in such a case.

So ...

> -	default:
> +	/* special types */
> +	case OPTION_END:
> +	case OPTION_GROUP:
> +	case OPTION_NUMBER:
> +	case OPTION_ALIAS:
>  		BUG("opt->type %d should not happen", opt->type);
>  	}
> +	return -1; /* gcc 10.2.1-6's -Werror=return-type */
>  }