diff mbox

assert: Suppress pedantic warning caused by statement expression [BZ# 21242]

Message ID 7635632a-e375-ea34-fe4f-19cd08262ce1@redhat.com
State Committed
Headers

Commit Message

Florian Weimer July 6, 2017, 9:50 a.m. UTC 
  On 07/05/2017 10:15 PM, Zack Weinberg wrote:
> On Wed, Jul 5, 2017 at 11:51 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 07/05/2017 05:46 PM, Zack Weinberg wrote:
>>> A problem occurs to me: expressions involving VLAs _are_ evaluated
>>> inside sizeof.
>>
>> The type of the sizeof argument would still be int (due to the
>> comparison against 0), so this doesn't actually occur.
> 
> I rechecked what C99 says about sizeof and VLAs, and you're right -
> the operand of sizeof is only evaluated when sizeof is _directly_
> applied to a VLA.  So this is indeed safe, but I think this wrinkle
> should be mentioned in the comment.  Perhaps
> 
> /* The first occurrence of EXPR is not evaluated due to the sizeof,
>    but will trigger any pedantic warnings masked by the __extension__
>    for the second occurrence.  The explicit comparison against zero
>    ensures that sizeof is not directly applied to a function pointer or
>    bit-field (which would be ill-formed) or VLA (which would be evaluated).  */
> 
> zw

What about the attached patch?

Siddhesh, is this okay during the freeze?  I'd like to backport it to
2.25 as well.

Thanks,
Florian

Comments

Florian Weimer Aug. 10, 2017, 3:32 p.m. UTC | #1 
On 07/06/2017 11:50 AM, Florian Weimer wrote:
> assert: Suppress pedantic warning caused by statement expression
> 
> 2017-07-06  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #21242]
> 	* assert/assert.h [__GNUC__ && !__STRICT_ANSI__] (assert):
> 	Suppress pedantic warning resulting from statement expression.
> 	(__ASSERT_FUNCTION): Add missing __extendsion__.

Ping?  <https://sourceware.org/ml/libc-alpha/2017-07/msg00227.html>

Thanks,
Florian
Zack Weinberg Aug. 10, 2017, 4:43 p.m. UTC | #2 
On Thu, Aug 10, 2017 at 11:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/06/2017 11:50 AM, Florian Weimer wrote:
>> assert: Suppress pedantic warning caused by statement expression

Looks good to me except ...

>>       (__ASSERT_FUNCTION): Add missing __extendsion__.

... typo here: __extendsion__ should be __extension__.

Might be good to confirm Joseph likes this version, tho.

zw
Joseph Myers Aug. 10, 2017, 10:28 p.m. UTC | #3 
On Thu, 10 Aug 2017, Zack Weinberg wrote:

> On Thu, Aug 10, 2017 at 11:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
> > On 07/06/2017 11:50 AM, Florian Weimer wrote:
> >> assert: Suppress pedantic warning caused by statement expression
> 
> Looks good to me except ...
> 
> >>       (__ASSERT_FUNCTION): Add missing __extendsion__.
> 
> ... typo here: __extendsion__ should be __extension__.
> 
> Might be good to confirm Joseph likes this version, tho.

I have no comments on this patch version.
diff mbox

Patch

assert: Suppress pedantic warning caused by statement expression

2017-07-06  Florian Weimer  <fweimer@redhat.com>

	[BZ #21242]
	* assert/assert.h [__GNUC__ && !__STRICT_ANSI__] (assert):
	Suppress pedantic warning resulting from statement expression.
	(__ASSERT_FUNCTION): Add missing __extendsion__.

diff --git a/assert/assert.h b/assert/assert.h
index 22f0195..6801cfe 100644
--- a/assert/assert.h
+++ b/assert/assert.h
@@ -91,13 +91,19 @@  __END_DECLS
      ? __ASSERT_VOID_CAST (0)						\
      : __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION))
 # else
+/* The first occurrence of EXPR is not evaluated due to the sizeof,
+   but will trigger any pedantic warnings masked by the __extension__
+   for the second occurrence.  The explicit comparison against zero is
+   required to support function pointers and bit fields in this
+   context, and to suppress the evaluation of variable length
+   arrays.  */
 #  define assert(expr)							\
-    ({									\
+  ((void) sizeof ((expr) == 0), __extension__ ({			\
       if (expr)								\
         ; /* empty */							\
       else								\
         __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION);	\
-    })
+    }))
 # endif
 
 # ifdef	__USE_GNU
@@ -113,7 +119,7 @@  __END_DECLS
    C9x has a similar variable called __func__, but prefer the GCC one since
    it demangles C++ function names.  */
 # if defined __cplusplus ? __GNUC_PREREQ (2, 6) : __GNUC_PREREQ (2, 4)
-#   define __ASSERT_FUNCTION	__PRETTY_FUNCTION__
+#   define __ASSERT_FUNCTION	__extension__ __PRETTY_FUNCTION__
 # else
 #  if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
 #   define __ASSERT_FUNCTION	__func__