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

[Jeff King <peff@peff.net>]

On Mon, Mar 10, 2025 at 11:50:20AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Maybe not often, if there is only one instance in the current code base.
> > Or maybe a lot, but we wouldn't know because we haven't had the warning
> > enabled.
> >
> > I guess another option is to enable it in _one_ CI job that uses clang
> > on Linux (maybe linux-sha256?) and see how often it is helpful or
> > harmful.
> 
> The reason why you said Linux rather than macOS is because the
> single instance we know about would not have to be worked around if
> we did it that way?
> 
> I am OK with that.

Yes, exactly. I started to prepare a patch for that, but then I realized
I'd probably be adding support in config.mak.dev. So we could also just
handle it automatically there, skipping the flag on macOS.

That would use the flag in more situations (blocking the known-bad case,
rather than enabling it in a known-good one). It might hit more false
positives, but I'd rather experiment in that direction and see if
anybody setting DEVELOPER=1 complains. After all, in either case it is
still a big question of whether this is the only false positive we'll
see, or if this is opening up a can of worms. So I consider it all
kind-of exploratory.

So that patch could look like this (on top of what you've queued already
in jk/use-wunreachable-code-for-devs).

-- >8 --
Subject: [PATCH] config.mak.dev: disable -Wunreachable-code on macOS

We've seen false positives here related to calling sigfillset(); even
though POSIX specifies that it may return an error, it transparently (to
the compiler) always returns success on macOS. As a result, the compiler
flags the error path in something like:

  if (sigfillset(&set))
	die(...);

as unreachable (which it is on this platform, but not in the general
case). We could work around it, but let's just disable the warning on
this platform. There are plenty of CI jobs that will still trigger it
(e.g., all of the linux+clang jobs).

Signed-off-by: Jeff King <peff@peff.net>
---
It's possible FreeBSD might share the same problem, but their manpage
does not seem to have the same "it always returns 0" language. But we
might need to expand this list if people report more problems.

 config.mak.dev | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/config.mak.dev b/config.mak.dev
index 95b7bc46ae..30dcd0c175 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -39,7 +39,12 @@ DEVELOPER_CFLAGS += -Wunused
 DEVELOPER_CFLAGS += -Wvla
 DEVELOPER_CFLAGS += -Wwrite-strings
 DEVELOPER_CFLAGS += -fno-common
+
+# There are false positives for unreachable code related to system
+# functions on macOS.
+ifneq ($(uname_S),Darwin)
 DEVELOPER_CFLAGS += -Wunreachable-code
+endif
 
 ifneq ($(filter clang4,$(COMPILER_FEATURES)),)
 DEVELOPER_CFLAGS += -Wtautological-constant-out-of-range-compare
-- 
2.49.0.rc2.384.gf2d6285ccb