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

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 14, 2025, 17:43 UTC
Message-ID: <Z9Rqx6CuXB_6JQoL@pks.im>
In-Reply-To: <xmqqr02zfr3r.fsf@gitster.g>

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

Show 26 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jeff King <peff@peff.net> writes:
> >
> >> -- >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.

Yeah, I'm also in favor of generally enabling the warning and seeing whether it will end up being a pain or not. This particular edge case here is ugly, but it's manageable and may protect us from mistakes in other places going forward.

If we do so, could we please also include the following patch for Meson?

Thanks!

Patrick

diff --git a/meson.build b/meson.build
index efe2871c9d..a0a602864a 100644
--- a/meson.build
+++ b/meson.build
@@ -721,6 +721,7 @@ if get_option('warning_level') in ['2','3', 'everything'] and compiler.get_argum
     '-Woverflow',
     '-Wpointer-arith',
     '-Wstrict-prototypes',
+    '-Wunreachable-code',
     '-Wunused',
     '-Wvla',
     '-Wwrite-strings',