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@xxxxxxxx>
> ---
> 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
