Re: [PATCH] config.mak.dev: enable -Wunreachable-code
From: Junio C Hamano <hidden>
Date: 2025-03-14 19:50:07
Jeff King [off-list ref] writes:
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))