Re: [PATCH 18/29] grep: catch a missing enum in switch statement

[Stefan Beller <sbeller@google.com>]

On Thu, May 11, 2017 at 1:08 PM, Brandon Williams <bmwill@google.com> wrote:
> On 05/11, Ævar Arnfjörð Bjarmason wrote:
>> Add a die(...) to a default case for the switch statement selecting
>> between grep pattern types under --recurse-submodules.
>>
>> Normally this would be caught by -Wswitch, but the grep_pattern_type
>> type is converted to int by going through parse_options(). Changing
>> the argument type passed to compile_submodule_options() won't work,
>> the value will just get coerced.
>>
>> Thus catching this at runtime is the least worst option. This won't
>> ever trigger in practice, but if a new pattern type were to be added
>> this catches an otherwise silent bug during development.
>>
>> See commit 0281e487fd ("grep: optionally recurse into submodules",
>> 2016-12-16) for the initial addition of this code.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  builtin/grep.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> index 3ffb5b4e81..1c0adb30f3 100644
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -495,6 +495,12 @@ static void compile_submodule_options(const struct grep_opt *opt,
>>               break;
>>       case GREP_PATTERN_TYPE_UNSPECIFIED:
>>               break;
>> +     default:
>> +             /*
>> +              * -Wswitch doesn't catch this due to casting &
>> +              * -Wswitch-default is too noisy.
>> +              */
>> +             die("BUG: Added a new grep pattern type without updating switch statement");

I am not sure if the comment is of enough value for the used screen
real estate value.
People interested in the existence of the default would just use
blame/log to find out.

Thanks,
Stefan