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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, May 15, 2017 at 7:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> 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. The -Wswitch-default warning will
>> warn about it, but that produces a lot of noise across the codebase,
>> this potential issue would be drowned in that noise.
>>
>> Thus catching this at runtime is the least worst option. This won't
>
> least bad, I guess?

Will fix.

>> ever trigger in practice, but if a new pattern type were to be added
>> this catches an otherwise silent bug during development.
>
> Good future-proofing.  When this and Peff's "BUG" thing both
> graduates, we can turn this into a BUG, I think.

Yup, this and a bunch of other stuff presumably. The BUG feature is nice.

> Thanks.
>
>>
>> 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 | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> index 3ffb5b4e81..a191e2976b 100644
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -495,6 +495,8 @@ static void compile_submodule_options(const struct grep_opt *opt,
>>               break;
>>       case GREP_PATTERN_TYPE_UNSPECIFIED:
>>               break;
>> +     default:
>> +             die("BUG: Added a new grep pattern type without updating switch statement");
>>       }
>>
>>       for (pattern = opt->pattern_list; pattern != NULL;