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

[Niels Möller]

Paul Eggert <eggert at cs.ucla.edu> writes:

> Mattias Engdegård reported that the Emacs master source currently
> generates the following warning when Emacs is built with mini-gmp.c
> under Clang 13:
>
>> In file included from /Users/mattias/emacs/lib/mini-gmp-gnulib.c:47:
>> /Users/mattias/emacs/lib/mini-gmp.c:1138:2: warning: unused variable '__cy' [-Wunused-variable]
>>         gmp_assert_nocarry (mpn_rshift (np, np, dn, shift));
>>         ^
>> /Users/mattias/emacs/lib/mini-gmp.c:91:15: note: expanded from macro 'gmp_assert_nocarry'
>>     mp_limb_t __cy = (x);          \
>
> The problem occurs because assert(X) does not evaluate X when NDEBUG
> is defined. Proposed patch attached.

Thanks for the report. I suspect builds with NDEBUG defined are not that
well tested.

> diff -r d45103d658ca mini-gmp/mini-gmp.c
> --- a/mini-gmp/mini-gmp.c	Wed Mar 30 23:16:18 2022 +0200
> +++ b/mini-gmp/mini-gmp.c	Fri Apr 08 16:18:06 2022 -0700
> @@ -87,10 +87,11 @@
>  #define GMP_MPN_OVERLAP_P(xp, xsize, yp, ysize)				\
>    ((xp) + (xsize) > (yp) && (yp) + (ysize) > (xp))
>  
> -#define gmp_assert_nocarry(x) do { \
> -    mp_limb_t __cy = (x);	   \
> -    assert (__cy == 0);		   \
> -  } while (0)
> +#ifdef NDEBUG
> +# define gmp_assert_nocarry(x) ((void) (x))
> +#else
> +# define gmp_assert_nocarry(x) assert ((x) == 0)
> +#endif

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.e., in builds without run-time asserts, ASSERT_NO_CARRY(X) expands to
just (X).

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

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)

Regards,
/Niels

-- 
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.