Re: [PATCH] assert.h: allow gcc to detect assert(a = 1) errors

[Jim Meyering <jim@meyering.net>]

On Wed, Nov 23, 2016 at 11:36 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/24/2016 03:21 AM, Jim Meyering wrote:
>
>> We *do* need that __STRICT_ANSI__ disjunct.
>> Otherwise, this would evoke no warning:
>>
>>   $ gcc -isystem. -I. -Werror=pedantic k.c
>>   In file included from k.c:1:0:
>>   k.c: In function ‘main’:
>>   k.c:2:23: warning: ISO C forbids braced-groups within expressions
>> [-Wpedantic]
>>    int main() { assert ( ({1;}) ); return 0; }
>
>
> Agreed.
>
>> Tests I ran manually in a directory with the new assert.h file:
>
>
>> Do you require a test suite addition for these? If so, would a single
>> bourne shell script be acceptable?
>
>
> We currently lack the machinery for that.  It's not just that it would need
> a shell script.  We also do not compile tests with headers as system
> headers.
>
> The patch looks good to me, but it needs a ChangeLog entry.

Thanks for the review.
Here's a proposed ChangeLog entry:

2016-11-25  Jim Meyering  <meyering@fb.com>

        Let gcc detect assert(a = 1) errors.
        * 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 assert-added parentheses.  The new
        definition uses "if (expr) /* empty */; else __assert_fail...",
        so gcc -Wall will now detect that type of error in an assert, too.
        The __STRICT_ANSI__ disjunct is to avoid the warning that -Wpedantic
        would otherwise issue for the use of ({...}).  I would have preferred
        to use __extension__ to mark that, but doing so would mistakenly
        suppress warnings about any extension in the user-supplied "expr".
        E.g., "assert ( ({1;}) )" must continue to evoke a warning.
        https://bugzilla.redhat.com/1105335