Re: [PATCH 0/13] more unused parameter cleanups

From: Jeff King <peff@peff.net>
Date: Mar 21, 2019, 09:44 UTC
Message-ID: <20190321094414.GA2894@sigill.intra.peff.net>
In-Reply-To: <87woksd322.fsf@evledraar.gmail.com>

On Thu, Mar 21, 2019 at 09:50:13AM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 8 quoted lines

> LGTM from skimming it, FWIW this is now what we need to compile cleanly
> with -Wextra:
> 
>     make DEVELOPER=1 DEVOPTS="extra-all" CFLAGS="-Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare -Wno-empty-body" all
> 
> For some such as -Wempty-body we'd really need to contort ourselves to
> get it passing anywhere near cleanly (all of those have existing "/*
> this is intentional! */" comments).

I think we could probably define a NOOP_BODY macro or function and use that instead. But it may not be worth the trouble. I'd have to see how painful that would be, and whether it might find any cases that actually look like real bugs.

For -Wunused-parameter I am working towards being able to actually enable that everywhere. It is not _too_ bad to annotate the instances which must be there, and my digging with it has uncovered several real bugs. Right now I'm in the "drop useless parameters" phase, which I expect will take one or two more rounds.

Then I'll start on the "annotate unused ones we must keep" series, which culminates in actually flipping on the switch with DEVELOPER (or rather, stopping flipping it off).

You can see my progress on the jk/unused-4-mark branch of https://github.com/peff/git (I think the contents are good, but the commit messages and organization need some cleanup).

Show 5 quoted lines

> I wonder if for the rest of these it's worth re-picking up this old
> suggestions of yours about #pragma:
> https://public-inbox.org/git/20170126143252.ne533mcv3n2ksbai@sigill.intra.peff.net/
> 
> I.e. for us to define our own macro for these cases & use it.

The push/pop ones may be of use (which both clang and gcc seem to support), since that would let us localize the effects. I think in many cases there's usually a more readable solution, though (e.g., you'd want to annotate specific parameters as unused with single word, not a 3-line push-diag/declare-param/pop-diag mess).

-Peff