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

Re: [PATCH 1/2] inline constant return from error() function

From
Junio C Hamano <gitster@pobox.com>
Date
May 6, 2014, 22:29 UTC
Message-ID
<xmqqppjqczhq.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140506151441.GA25768@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> Commit e208f9c introduced a macro to turn error() calls
> into:
>
>   (error(), -1)
>
> to make the constant return value more visible to the
> calling code (and thus let the compiler make better
> decisions about the code).
>
> This works well for code like:
>
>   return error(...);
>
> but the "-1" is superfluous in code that just calls error()
> without caring about the return value. In older versions of
> gcc, that was fine, but gcc 4.9 complains with -Wunused-value.
>
> We can work around this by encapsulating the constant return
> value in a static inline function, as gcc specifically
> avoids complaining about unused function returns unless the
> function has been specifically marked with the
> warn_unused_result attribute.

That's kind of W*A*T magic, and I generally try to avoid magic, as long as it solves your "can we make both -O2 with new compilers and -O3 happy?" I wouldn't complain ;-)

Show 12 quoted lines
> We also use the same trick for config_error_nonbool and
> opterror, which learned the same error technique in a469a10.
>
> Reported-by: Felipe Contreras <felipe.contreras@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> On Mon, May 05, 2014 at 05:29:38PM -0400, Jeff King wrote:
>
>> I cannot think of any other way to make the compiler aware of the
>> constant value, but perhaps somebody else is more clever than I am.
>
> This came to me in a dream, and seems to work.
Show 49 quoted lines
>
>  cache.h           | 2 +-
>  git-compat-util.h | 6 +++++-
>  parse-options.h   | 2 +-
>  3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 107ac61..e2f12b0 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1272,7 +1272,7 @@ extern int git_env_bool(const char *, int);
>  extern int git_config_system(void);
>  extern int config_error_nonbool(const char *);
>  #if defined(__GNUC__) && ! defined(__clang__)
> -#define config_error_nonbool(s) (config_error_nonbool(s), -1)
> +#define config_error_nonbool(s) (config_error_nonbool(s), const_error())
>  #endif
>  extern const char *get_log_output_encoding(void);
>  extern const char *get_commit_output_encoding(void);
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f6d3a46..b4c437e 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -331,7 +331,11 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
>   * using the function as usual.
>   */
>  #if defined(__GNUC__) && ! defined(__clang__)
> -#define error(...) (error(__VA_ARGS__), -1)
> +static inline int const_error(void)
> +{
> +	return -1;
> +}
> +#define error(...) (error(__VA_ARGS__), const_error())
>  #endif
>  
>  extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
> diff --git a/parse-options.h b/parse-options.h
> index 3189676..2f9be96 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -177,7 +177,7 @@ extern NORETURN void usage_msg_opt(const char *msg,
>  extern int optbug(const struct option *opt, const char *reason);
>  extern int opterror(const struct option *opt, const char *reason, int flags);
>  #if defined(__GNUC__) && ! defined(__clang__)
> -#define opterror(o,r,f) (opterror((o),(r),(f)), -1)
> +#define opterror(o,r,f) (opterror((o),(r),(f)), const_error())
>  #endif
>  
>  /*----- incremental advanced APIs -----*/
Previous: Jeff KingNext: Jeff King
Message 11 of 26 in “Fix a ton of compiler warnings”
  1. 0/3 Fix a ton of compiler warningsFelipe Contreras, May 4, 2014
  2. 1/3 Revert "make error()'s constant return value more visible"Felipe Contreras, May 4, 2014
  3. Jeff KingMay 5, 2014
  4. Felipe ContrerasMay 5, 2014
  5. Jeff KingMay 5, 2014
  6. Felipe ContrerasMay 5, 2014
  7. Jeff KingMay 5, 2014
  8. Felipe ContrerasMay 5, 2014
  9. Jeff KingMay 5, 2014
  10. 1/2 inline constant return from error() functionJeff King, May 6, 2014
  11. Junio C HamanoMay 6, 2014
  12. Jeff KingMay 7, 2014
  13. Junio C HamanoMay 11, 2014
  14. Jeff KingMay 12, 2014
  15. Felipe ContrerasMay 11, 2014
  16. Jonathan NiederMay 12, 2014
  17. Jeff KingMay 12, 2014
  18. 2/2 let clang use the constant-return error() macroJeff King, May 6, 2014
  19. 2/3 Revert "silence some -Wuninitialized false positives"Felipe Contreras, May 4, 2014
  20. 3/3 Silence a bunch of format-zero-length warningsFelipe Contreras, May 4, 2014
  21. brian m. carlsonMay 4, 2014
  22. Felipe ContrerasMay 4, 2014
  23. Jeff KingMay 5, 2014
  24. Junio C HamanoMay 7, 2014
  25. Heiko VoigtMay 7, 2014
  26. Junio C HamanoMay 7, 2014

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.