git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Calvin Wan <calvinwan@google.com>
Date
Mar 18, 2025, 22:04 UTC
Message-ID
<20250318220453.1937685-1-calvinwan@google.com>
In-Reply-To
<20250317235329.809302-3-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 64 quoted lines
> 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`.

Previous: Junio C HamanoNext: Calvin Wan
Message 48 of 59 in “refs: introduce support for partial reference transactions”
  1. 0/8 refs: introduce support for partial reference transactionsKarthik Nayak, Mar 5, 2025
  2. 1/8 refs/files: remove redundant check in split_symref_update()Karthik Nayak, Mar 5, 2025
  3. Junio C HamanoMar 5, 2025
  4. Karthik NayakMar 6, 2025
  5. 3/8 refs/files: remove duplicate duplicates checkKarthik Nayak, Mar 5, 2025
  6. 2/8 refs: move duplicate refname update check to generic layerKarthik Nayak, Mar 5, 2025
  7. Junio C HamanoMar 5, 2025
  8. Karthik NayakMar 6, 2025
  9. 4/8 refs/reftable: extract code from the transaction preparationKarthik Nayak, Mar 5, 2025
  10. 5/8 refs: introduce enum-based transaction error typesKarthik Nayak, Mar 5, 2025
  11. 6/8 refs: implement partial reference transaction supportKarthik Nayak, Mar 5, 2025
  12. Jeff KingMar 7, 2025
  13. Junio C HamanoMar 7, 2025
  14. Junio C HamanoMar 7, 2025
  15. Karthik NayakMar 7, 2025
  16. config.mak.dev: enable -Wunreachable-codeJeff King, Mar 7, 2025
  17. Junio C HamanoMar 7, 2025
  18. Jeff KingMar 8, 2025
  19. Junio C HamanoMar 10, 2025
  20. Jeff KingMar 10, 2025
  21. Junio C HamanoMar 10, 2025
  22. Jeff KingMar 14, 2025
  23. Jeff KingMar 14, 2025
  24. Junio C HamanoMar 14, 2025
  25. Junio C HamanoMar 14, 2025
  26. Patrick SteinhardtMar 14, 2025
  27. Jeff KingMar 14, 2025
  28. Junio C HamanoMar 14, 2025
  29. Junio C HamanoMar 14, 2025
  30. Mike HommeyJun 3, 2025
  31. Junio C HamanoJun 3, 2025
  32. Mike HommeyJun 3, 2025
  33. Mike HommeyJun 3, 2025
  34. 0/3 -Wunreachable-codeJunio C Hamano, Mar 14, 2025
  35. 1/3 config.mak.dev: enable -Wunreachable-codeJunio C Hamano, Mar 14, 2025
  36. 2/3 run-command: use errno to check for sigfillset() errorJunio C Hamano, Mar 14, 2025
  37. Taylor BlauMar 17, 2025
  38. Junio C HamanoMar 17, 2025
  39. Junio C HamanoMar 18, 2025
  40. 3/3 git-compat-util: add NOT_A_CONST macro and use it in atfork_prepare()Junio C Hamano, Mar 14, 2025
  41. Junio C HamanoMar 14, 2025
  42. Jeff KingMar 17, 2025
  43. 0/3 -Wunreachable-codeJunio C Hamano, Mar 17, 2025
  44. 1/3 run-command: use errno to check for sigfillset() errorJunio C Hamano, Mar 17, 2025
  45. 2/3 git-compat-util: add NOT_CONSTANT macro and use it in atfork_prepare()Junio C Hamano, Mar 17, 2025
  46. Jeff KingMar 18, 2025
  47. Junio C HamanoMar 18, 2025
  48. Calvin WanMar 18, 2025
  49. Calvin WanMar 18, 2025
  50. Junio C HamanoMar 18, 2025
  51. 3/3 config.mak.dev: enable -Wunreachable-codeJunio C Hamano, Mar 17, 2025
  52. Jeff KingMar 18, 2025
  53. Karthik NayakMar 7, 2025
  54. Jeff KingMar 7, 2025
  55. Karthik NayakMar 7, 2025
  56. 7/8 refs: support partial update rejections during F/D checksKarthik Nayak, Mar 5, 2025
  57. 8/8 update-ref: add --allow-partial flag for stdin modeKarthik Nayak, Mar 5, 2025
  58. Junio C HamanoMar 5, 2025
  59. Karthik NayakMar 6, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.