Re: [PATCH] config.mak.dev: enable -Wunreachable-code

From: Jeff King <peff@peff.net>
Date: Mar 14, 2025, 18:53 UTC
Message-ID: <20250314185325.GC578421@coredump.intra.peff.net>
In-Reply-To: <xmqqr02zfr3r.fsf@gitster.g>

On Fri, Mar 14, 2025 at 10:40:24AM -0700, Junio C Hamano wrote:

Show 22 quoted lines

> >> -- >8 --
> >> Subject: [PATCH] run-command: use errno to check for sigfillset() error
> >>
> >> Since enabling -Wunreachable-code, builds with clang on macOS now fail,
> >> complaining that the die_errno() call in:
> >>
> >>   if (sigfillset(&all))
> >> 	die_errno("sigfillset");
> >>
> >> is unreachable. On that platform the manpage documents that sigfillset()
> >> always returns success, and presumably the implementation is a macro or
> >> inline function that does so in a way that is transparent to the
> >> compiler.
> >
> > Would it work to instead do this here
> > ...
> 
> I forgot to say a more important thing.  Between the "let's excempt
> developers on macOS" and the "let's see how far we can go with the
> warning turned on everywhere and wack-a-mole this particular one
> with errno check" patches, I prefer the latter at least for a short
> term.

That's my gut feeling, too. I wasn't sure how people would feel about actually touching the code (whereas the other patches were purely turning compiler knobs). It may turn into wack-a-mole, but finding out is part of the experiment.

Your CAN_BE_TAKEN() approach is certainly less subtle, and can be applied in a more general way. If this is the only spot needed it may be overkill, but the readability improvement alone probably makes it worthwhile.

Do you want to turn that into a patch?

-Peff