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

> 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.