Re: [PATCH 6/6] config.mak.dev: enable -Wunused-parameter by default

[Patrick Steinhardt <ps@pks.im>]

On Wed, Aug 28, 2024 at 01:56:13AM -0400, Eric Sunshine wrote:
> On Wed, Aug 28, 2024 at 12:01 AM Jeff King <peff@peff.net> wrote:
> > Having now removed or annotated all of the unused function parameters in
> > our code base, I found that each instance falls into one of three
> > categories:
> >
> >   1. ignoring the parameter is a bug (e.g., a function takes a ptr/len
> >      pair, but ignores the length). Detecting these helps us find the
> >      bugs.
> >
> >   2. the parameter is unnecessary (and usually left over from a
> >      refactoring or earlier iteration of a patches series). Removing
> >      these cleans up the code.
> >
> >   3. the function has to conform to a specific interface (because it's
> >      used via a function pointer, or matches something on the other side
> >      of an #ifdef). These ones are annoying, but annotating them with
> >      UNUSED is not too bad (especially if the compiler tells you about
> >      the problem promptly).
> > [...]
> > And since the code base is now at a spot where we compile cleanly with
> > -Wunused-parameter, turning it on will make it the responsibility of
> > individual patch writers going forward.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> > diff --git a/config.mak.dev b/config.mak.dev
> > @@ -54,7 +54,6 @@ ifeq ($(filter extra-all,$(DEVOPTS)),)
> >  DEVELOPER_CFLAGS += -Wno-sign-compare
> > -DEVELOPER_CFLAGS += -Wno-unused-parameter
> 
> What is the expectation regarding newcomers to the project or even
> people who have not been following this topic and its cousins?
> Documentation/CodingGuidelines recommends enabling DEVELOPER mode,
> which is good, but this change means that such people may now be hit
> with a compiler complaint which they don't necessarily know how to
> deal with in the legitimate case #3 (described above). Should
> CodingGuidelines be updated to mention "UNUSED" and the circumstances
> under which it should be used?

Updating our coding guidelines would certainly be welcome. Other than
that this series looks good to me and is a step into the right direction
in my opinion. Thanks!

Patrick