clang warning about mini-gmp.c when used in Emacs

[Vincent Lefevre]

On 2022-04-15 07:29:34 +0200, Niels Möller wrote:
> I have two minor comments on this proposed fix:
> 
> 1. Is the void cast really needed? Corresponding macros in gmp-impl.h
> are defined like
> 
>   #if WANT_ASSERT
>   #define ASSERT_CARRY(expr)     ASSERT_ALWAYS ((expr) != 0)
>   #define ASSERT_NOCARRY(expr)   ASSERT_ALWAYS ((expr) == 0)
>   #else
>   #define ASSERT_CARRY(expr)     (expr)
>   #define ASSERT_NOCARRY(expr)   (expr)
>   #endif

I'd say that a cast to void is better in order to ensure that
ASSERT_CARRY(expr) and ASSERT_NOCARRY(expr) have type void
whether WANT_ASSERT is defined or not, so that type issues
could be detected more easily.

> 2. I'm a bit uneasy about side effects in the argument to the standard
> assert macro, even with the NDEBUG ifdefs.

Ditto. It seems that the ISO C standard doesn't say anything if
NDEBUG not defined (the expression might be evaluated several
times, like with any macro).

> Alternative patch:
> 
> --- a/mini-gmp/mini-gmp.c	Tue Feb 15 09:18:40 2022 +0100
> +++ b/mini-gmp/mini-gmp.c	Fri Apr 15 07:20:40 2022 +0200
> @@ -90,6 +90,7 @@ see https://www.gnu.org/licenses/.  */
>  #define gmp_assert_nocarry(x) do { \
>      mp_limb_t __cy = (x);	   \
>      assert (__cy == 0);		   \
> +    (void) __cy;		   \
>    } while (0)

I prefer this patch.

-- 
Vincent Lefèvre <vincent at vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)