Re: [PATCH v3 2/3] git-compat-util: add NOT_CONSTANT macro and use it in atfork_prepare()

[Calvin Wan <calvinwan@google.com>]

Junio C Hamano <gitster@pobox.com> writes:
> Our hope is that the number of code paths that falsely trigger
> warnings with the -Wunreachable-code compilation option are small,
> and they can be worked around case-by-case basis, like we just did
> in the previous commit.  If we need such a workaround a bit more
> often, however, we may benefit from a more generic and descriptive
> facility that helps document the cases we need such workarounds.
> 
>     Side note: if we need the workaround all over the place, it
>     simply means -Wunreachable-code is not a good tool for us to
>     save engineering effort to catch mistakes.  We are still
>     exploring if it helps us, so let's assume that it is not the
>     case.
> 
> Introduce NOT_CONSTANT() macro, with which, the developer can tell
> the compiler:
> 
>     Do not optimize this expression out, because, despite whatever
>     you are told by the system headers, this expression should *not*
>     be treated as a constant.
> 
> and use it as a replacement for the workaround we used that was
> somewhat specific to the sigfillset case.  If the compiler already
> knows that the call to sigfillset() cannot fail on a particular
> platform it is compiling for and declares that the if() condition
> would not hold, it is plausible that the next version of the
> compiler may learn that sigfillset() that never fails would not
> touch errno and decide that in this sequence:
> 
> 	errno = 0;
> 	sigfillset(&all)
> 	if (errno)
> 		die_errno("sigfillset");
> 
> the if() statement will never trigger.  Marking that the value
> returned by sigfillset() cannot be a constant would document our
> intention better and would not break with such a new version of
> compiler that is even more "clever".  With the marco, the above
> sequence can be rewritten:
> 
> 	if (NOT_CONSTANT(sigfillset(&all)))
> 		die_errno("sigfillset");
> 
> which looks almost like other innocuous annotations we have,
> e.g. UNUSED.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Makefile                         |  1 +
>  compiler-tricks/not-a-constant.c |  2 ++
>  git-compat-util.h                |  9 +++++++++
>  meson.build                      |  1 +
>  run-command.c                    | 12 +++++-------
>  5 files changed, 18 insertions(+), 7 deletions(-)
>  create mode 100644 compiler-tricks/not-a-constant.c
> 
> diff --git a/Makefile b/Makefile
> index 97e8385b66..605e2d7f61 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -985,6 +985,7 @@ LIB_OBJS += compat/nonblock.o
>  LIB_OBJS += compat/obstack.o
>  LIB_OBJS += compat/terminal.o
>  LIB_OBJS += compat/zlib-uncompress2.o
> +LIB_OBJS += compiler-tricks/not-a-constant.o

The name is correctly added here, but in `next,` this name is set to
`compiler-tricks/not-constant.o`.