Re: [PATCH] config.mak.dev: enable -Wunreachable-code
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 14, 2025, 19:50 UTC
- Message-ID
- <xmqq7c4rfl3o.fsf@gitster.g>
- In-Reply-To
- <20250314185325.GC578421@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> 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?
Yes, but after I come up with a better name. CAN_BE_TAKEN may be OK for if/while but not good enough for switch() for example. "Do not opmimize out because, despite your beliefs, this expression is ..." is what we want to convey.
Makefile | 1 + git-compat-util.h | 9 +++++++++ meson.build | 1 + run-command.c | 12 +++++------- 4 files changed, 16 insertions(+), 7 deletions(-)
diff --git c/Makefile w/Makefile index 97e8385b66..2158bf6916 100644 --- c/Makefile +++ w/Makefile @@ -1018,6 +1018,7 @@ LIB_OBJS += ewah/ewah_bitmap.o LIB_OBJS += ewah/ewah_io.o LIB_OBJS += ewah/ewah_rlw.o LIB_OBJS += exec-cmd.o +LIB_OBJS += fbtcdnki.o LIB_OBJS += fetch-negotiator.o LIB_OBJS += fetch-pack.o LIB_OBJS += fmt-merge-msg.o diff --git c/git-compat-util.h w/git-compat-util.h index e283c46c6f..63a3ef6b70 100644 --- c/git-compat-util.h +++ w/git-compat-util.h @@ -1593,4 +1593,13 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset) ((uintptr_t)&(ptr)->member - (uintptr_t)(ptr)) #endif /* !__GNUC__ */ +/* + * Prevent an overly clever compiler from optimizing an expression + * out, triggering a false positive when building with the + * -Wunreachable-code option. false_but_the_compiler_does_not_know_it_ + * is defined in a compilation unit separate from where the macro is + * used, initialized to 0, and never modified. + */ +#define NOT_A_CONST(expr) ((expr) || false_but_the_compiler_does_not_know_it_) +extern int false_but_the_compiler_does_not_know_it_; #endif diff --git c/meson.build w/meson.build index f60f3f49e4..ce642dcf65 100644 --- c/meson.build +++ w/meson.build @@ -282,6 +282,7 @@ libgit_sources = [ 'ewah/ewah_io.c', 'ewah/ewah_rlw.c', 'exec-cmd.c', + 'fbtcdnki.c', 'fetch-negotiator.c', 'fetch-pack.c', 'fmt-merge-msg.c', diff --git c/run-command.c w/run-command.c index d527c46175..535c73a059 100644 --- c/run-command.c +++ w/run-command.c @@ -516,14 +516,12 @@ static void atfork_prepare(struct atfork_state *as) sigset_t all; /* - * Do not use the return value of sigfillset(). It is transparently 0 - * on some platforms, meaning a clever compiler may complain that - * the conditional body is dead code. Instead, check for error via - * errno, which outsmarts the compiler. + * POSIX says sitfillset() can fail, but an overly clever + * compiler can see through the header files and decide + * it cannot fail on a particular platform it is compiling for, + * triggering -Wunreachable-code false positive. */ - errno = 0; - sigfillset(&all); - if (errno) + if (NOT_A_CONST(sigfillset(&all))) die_errno("sigfillset"); #ifdef NO_PTHREADS if (sigprocmask(SIG_SETMASK, &all, &as->old))