[PATCH] Tweak assert -DNDEBUG for the new -Wunused-but-set-{variable,parameter} warnings

[PATCH] Tweak assert -DNDEBUG for the new -Wunused-but-set-{variable,parameter} warnings

[Jakub Jelinek]

Hi!

I've yesterday committed to GCC trunk a patch to support
new -Wunused-but-set-{variable,parameter} warnings where GCC
warns about variables that have been just set but not really
used.

This can create unwanted warnings with current definition of assert
and -DNDEBUG - people often do something like:

void
foo (void)
{
  int r;
  r = bar ();
  assert (r != 6);
}

The variable is set and used without -DNDEBUG, but with it if
assert expands just to (void) 0 then it is just set but not used.

The following patch shouldn't result in worse code generation (not
even with -O0 I believe) with -DNDEBUG, should quiet that warning
and also allow some checking of expr.  With -DNDEBUG assert will
happily accept even syntax errors within its arguments etc., which
would only show up without -DNDEBUG, this patch ought to cure that
too.  Alternative to the 0 && (expr) could be sizeof (expr) or
something else where the expression isn't evaluated, but is parsed.

2010-04-08  Jakub Jelinek  <jakub@redhat.com>

	* assert/assert.h [NDEBUG] (assert, assert_perror): Include
	the argument after "0 &&".

--- libc/assert/assert.h.jj	2009-05-16 19:23:29.000000000 +0200
+++ libc/assert/assert.h	2010-04-08 12:10:06.000000000 +0200
@@ -1,4 +1,4 @@
-/* Copyright (C) 1991,1992,1994-2001,2003,2004,2007
+/* Copyright (C) 1991,1992,1994-2001,2003,2004,2007,2010
    Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -49,7 +49,7 @@
 
 #ifdef	NDEBUG
 
-# define assert(expr)		(__ASSERT_VOID_CAST (0))
+# define assert(expr)		(__ASSERT_VOID_CAST (0 && (expr)))
 
 /* void assert_perror (int errnum);
 
@@ -58,7 +58,7 @@
    (This is a GNU extension.) */
 
 # ifdef	__USE_GNU
-#  define assert_perror(errnum)	(__ASSERT_VOID_CAST (0))
+#  define assert_perror(errnum)	(__ASSERT_VOID_CAST (0 && (errnum)))
 # endif
 
 #else /* Not NDEBUG.  */


	Jakub

Re: [PATCH] Tweak assert -DNDEBUG for the new -Wunused-but-set-{variable,parameter} warnings

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> The following patch shouldn't result in worse code generation (not
> even with -O0 I believe) with -DNDEBUG, should quiet that warning
> and also allow some checking of expr.  With -DNDEBUG assert will
> happily accept even syntax errors within its arguments etc., which
> would only show up without -DNDEBUG, this patch ought to cure that
> too.  Alternative to the 0 && (expr) could be sizeof (expr) or
> something else where the expression isn't evaluated, but is parsed.

I don't think this is allowed, as the standard explicitly says that the
argument is ignored when NDEBUG is defined.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [PATCH] Tweak assert -DNDEBUG for the new -Wunused-but-set-{variable,parameter} warnings

[Roland McGrath]

Unfortunately I think this is wrongly incompatible, as Andreas said.
e.g., consider:

#ifndef NDEBUG
static int checker () { ... }
#endif

...
	assert (checker () == 0);
...

Or even worse, it could be hairy macros so that under NDEBUG
the expression is actually unparseable.


Thanks,
Roland