assert.h: allow gcc to detect assert(a = 1) errors

  From: Jim Meyering <meyering@fb.com>

* assert/assert.h (assert): Rewrite assert's definition so that
a s/==/=/ typo, e.g., assert(errno = ENOENT) is not hidden from
gcc's -Wparentheses by the parentheses added by assert.
The new definition uses "if (expr) ; else __assert_fail...", so
gcc -Wall will now detect that type of error in an assert, too.
---
 assert/assert.h | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
  

This certainly should not go in during the current release freeze.
So you might not want to try to get too much discussion going until
the floodgates reopen.  Also, various key people might be less than
usually responsive until after Cauldron (and for me, another week of
vacation thereafter).

Isn't there an "empty statement" warning that might be on too?  We
certainly don't want the internals of the macro (the then clause in
your new version) to produce warnings of their own with any possible
combination of switches.

You should be able to use __extension__ for GCC under -ansi.  But
perhaps that would wrongly allow use of extensions inside the user's
expression too.  If you're avoiding it for good reason, there should
be comments there explaining why.

I also wonder how modern GCCs running in integrated preprocessor
mode deal with the system header exception and/or __extension__ for
this sort of case (since in integrated preprocessor mode it can tell
which part came from the system header and which from the user).

Any change like this is going to need a lot of detailed reporting
about testing with different compilers and option combinations and
sorts of expressions (i.e. ones using extensions or not and perhaps
various different sorts of extensions).


Thanks,
Roland
  

On Wed, Jul 16, 2014 at 1:15 PM, Roland McGrath <roland@hack.frob.com> wrote:
> This certainly should not go in during the current release freeze.
> So you might not want to try to get too much discussion going until
> the floodgates reopen.  Also, various key people might be less than
> usually responsive until after Cauldron (and for me, another week of
> vacation thereafter).

Thanks for the quick feedback.

> Isn't there an "empty statement" warning that might be on too?  We
> certainly don't want the internals of the macro (the then clause in
> your new version) to produce warnings of their own with any possible
> combination of switches.

-Wempty-body does not complain about the empty "then" clause
inside that statement-expression.

> You should be able to use __extension__ for GCC under -ansi.  But
> perhaps that would wrongly allow use of extensions inside the user's
> expression too.  If you're avoiding it for good reason, there should
> be comments there explaining why.

Good point. I confirmed that prefixing the ({... with __extension__
lets me eliminate the __STRICT_ANSI__ disjunct, and that -ansi
still warns about a use like "assert( ({1}) );".  I'll update the patch
and repost here -- with a detailed test description --
in a couple weeks.

> I also wonder how modern GCCs running in integrated preprocessor
> mode deal with the system header exception and/or __extension__ for
> this sort of case (since in integrated preprocessor mode it can tell
> which part came from the system header and which from the user).
>
> Any change like this is going to need a lot of detailed reporting
> about testing with different compilers and option combinations and
> sorts of expressions (i.e. ones using extensions or not and perhaps
> various different sorts of extensions).

I've tested it with -Wextra -Wall -W -O, with uses as statements and
as expressions.  Also with and without -ansi.  I have not yet tested
with a non-__GNUC__ compiler, but will do.

Thanks,
Jim