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

[Jim Meyering <jim at meyering dot net>]

On Fri, Nov 25, 2016 at 10:14 PM, Jim Meyering <jim@meyering.net> wrote:
> 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

Here's the complete, rebased patch. Ok to push, presuming I still have
commit access?

Attachment: glibc-assert--do-not-suppress-warning-about-accidental-assignment.diff
Description: Text document