Re: [PATCH] config.mak.dev: enable -Wunused-function

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 18/10/2018 08:05, Jeff King wrote:
> We explicitly omitted -Wunused-function when we added
> -Wextra, but there is no need: the current code compiles
> cleanly with it. And it's worth having, since it can let you
> know when there are cascading effects from a cleanup (e.g.,
> deleting one function lets you delete its static helpers).
> 
> There are cases where we may need an unused function to
> exist, but we can handle these easily:
> 
>   - macro-generated code like commit-slab; there we have the
>     MAYBE_UNUSED annotation to silence the compiler
> 
>   - conditional compilation, where we may or may not need a
>     static helper. These generally fall into one of two
>     categories:
> 
>       - the call should not be conditional, but rather the
> 	function body itself should be (and may just be a
> 	no-op on one side of the #if). That keeps the
> 	conditional pollution out of the main code.
> 
>       - call-chains of static helpers should all be in the
>         same #if block, so they are all-or-nothing
> 
>     And if there's some case that doesn't cover, we still
>     have MAYBE_UNUSED as a fallback.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  config.mak.dev | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/config.mak.dev b/config.mak.dev
> index 92d268137f..bbeeff44fe 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -34,7 +34,6 @@ ifeq ($(filter extra-all,$(DEVOPTS)),)
>  CFLAGS += -Wno-empty-body
>  CFLAGS += -Wno-missing-field-initializers
>  CFLAGS += -Wno-sign-compare
> -CFLAGS += -Wno-unused-function
>  CFLAGS += -Wno-unused-parameter
>  endif
>  endif
> 

Heh, as luck would have it, tonight I had an -Wunused-function
warning on cygwin, but not Linux, when building the 'pu' branch.

[On linux, I am using DEVELOPER=1 in my config.mak etc., whereas
on cygwin I am still using an explicit (but very similar) list
of -W args.]

I haven't looked too deeply, but this seems to be caused by
Junio's commit 42c89ea70a ("SQUASH??? - convert the other user of
string-list as db", 2018-10-17) which removes a call to the
add_existing() function - the subject of the warning.

[BTW there is another 'static add_existing()' in builtin/show_ref.c]

ATB,
Ramsay Jones