On Thu, May 11, 2017 at 10:40 PM, Stefan Beller <[email protected]> wrote: > On Thu, May 11, 2017 at 1:08 PM, Brandon Williams <[email protected]> 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 <[email protected]> >>> --- >>> 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, I was on the fence about it, will remove it (and add the mention of -Wswitch-default to the commit message).
