Emil Velikov <emil.l.veli...@gmail.com> writes:

> As we evaluate sizeof() at compile time, having the run-time assert()
> does not provide any benefits. Move to the compile-time version
> STATIC_ASSERT() which will kindly prompt us when this go wrong.
>

This doesn't look right.  AFAIK STATIC_ASSERT() is implemented by
expanding to an array type-id of negative size, which is an error
regardless of whether the sizeof expression is evaluated or not: i.e.
"0 ? sizeof(invalid) : ..." is still an error for the same reason that
"0 * sizeof(invalid)" or "(void)sizeof(invalid)" is an error.  That and
also that the whole thing is wrapped in a do-while loop you cannot use
as operand of the ternary operator...

> Cc: Matt Turner <matts...@gmail.com>
> Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
> ---
>  src/util/u_atomic.h | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/src/util/u_atomic.h b/src/util/u_atomic.h
> index e38395a..10c0cca 100644
> --- a/src/util/u_atomic.h
> +++ b/src/util/u_atomic.h
> @@ -86,7 +86,7 @@
>  #endif
>  #include <windows.h>
>  #include <intrin.h>
> -#include <assert.h>
> +#include "util/macros.h"
>  
>  #if _MSC_VER < 1600
>  
> @@ -166,7 +166,7 @@ _InterlockedExchangeAdd8(char volatile *addend, char 
> value)
>     sizeof *(_v) == sizeof(short)   ? _InterlockedIncrement16((short *)  
> (_v)) : \
>     sizeof *(_v) == sizeof(long)    ? _InterlockedIncrement  ((long *)   
> (_v)) : \
>     sizeof *(_v) == sizeof(__int64) ? InterlockedIncrement64 ((__int64 
> *)(_v)) : \
> -                                     (assert(!"should not get here"), 0))
> +                                     (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_dec(_v) \
>     ((void) p_atomic_dec_return(_v))
> @@ -175,21 +175,21 @@ _InterlockedExchangeAdd8(char volatile *addend, char 
> value)
>     sizeof *(_v) == sizeof(short)   ? _InterlockedDecrement16((short *)  
> (_v)) : \
>     sizeof *(_v) == sizeof(long)    ? _InterlockedDecrement  ((long *)   
> (_v)) : \
>     sizeof *(_v) == sizeof(__int64) ? InterlockedDecrement64 ((__int64 
> *)(_v)) : \
> -                                     (assert(!"should not get here"), 0))
> +                                     (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_add(_v, _i) (\
>     sizeof *(_v) == sizeof(char)    ? _InterlockedExchangeAdd8 ((char *)   
> (_v), (_i)) : \
>     sizeof *(_v) == sizeof(short)   ? _InterlockedExchangeAdd16((short *)  
> (_v), (_i)) : \
>     sizeof *(_v) == sizeof(long)    ? _InterlockedExchangeAdd  ((long *)   
> (_v), (_i)) : \
>     sizeof *(_v) == sizeof(__int64) ? InterlockedExchangeAdd64((__int64 
> *)(_v), (_i)) : \
> -                                     (assert(!"should not get here"), 0))
> +                                     (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_cmpxchg(_v, _old, _new) (\
>     sizeof *(_v) == sizeof(char)    ? _InterlockedCompareExchange8 ((char *)  
>  (_v), (char)   (_new), (char)   (_old)) : \
>     sizeof *(_v) == sizeof(short)   ? _InterlockedCompareExchange16((short *) 
>  (_v), (short)  (_new), (short)  (_old)) : \
>     sizeof *(_v) == sizeof(long)    ? _InterlockedCompareExchange  ((long *)  
>  (_v), (long)   (_new), (long)   (_old)) : \
>     sizeof *(_v) == sizeof(__int64) ? InterlockedCompareExchange64 ((__int64 
> *)(_v), (__int64)(_new), (__int64)(_old)) : \
> -                                     (assert(!"should not get here"), 0))
> +                                     (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #endif
>  
> @@ -198,7 +198,7 @@ _InterlockedExchangeAdd8(char volatile *addend, char 
> value)
>  #define PIPE_ATOMIC "Solaris OS atomic functions"
>  
>  #include <atomic.h>
> -#include <assert.h>
> +#include "util/macros.h"
>  
>  #define p_atomic_set(_v, _i) (*(_v) = (_i))
>  #define p_atomic_read(_v) (*(_v))
> @@ -208,49 +208,49 @@ _InterlockedExchangeAdd8(char volatile *addend, char 
> value)
>     sizeof(*v) == sizeof(uint16_t) ? atomic_dec_16_nv((uint16_t *)(v)) == 0 : 
> \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_dec_32_nv((uint32_t *)(v)) == 0 : 
> \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_dec_64_nv((uint64_t *)(v)) == 0 : 
> \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_inc(v) (void) (\
>     sizeof(*v) == sizeof(uint8_t)  ? atomic_inc_8 ((uint8_t  *)(v)) : \
>     sizeof(*v) == sizeof(uint16_t) ? atomic_inc_16((uint16_t *)(v)) : \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_inc_32((uint32_t *)(v)) : \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_inc_64((uint64_t *)(v)) : \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_inc_return(v) ((__typeof(*v)) \
>     sizeof(*v) == sizeof(uint8_t)  ? atomic_inc_8_nv ((uint8_t  *)(v)) : \
>     sizeof(*v) == sizeof(uint16_t) ? atomic_inc_16_nv((uint16_t *)(v)) : \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_inc_32_nv((uint32_t *)(v)) : \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_inc_64_nv((uint64_t *)(v)) : \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_dec(v) ((void) \
>     sizeof(*v) == sizeof(uint8_t)  ? atomic_dec_8 ((uint8_t  *)(v)) : \
>     sizeof(*v) == sizeof(uint16_t) ? atomic_dec_16((uint16_t *)(v)) : \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_dec_32((uint32_t *)(v)) : \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_dec_64((uint64_t *)(v)) : \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_dec_return(v) ((__typeof(*v)) \
>     sizeof(*v) == sizeof(uint8_t)  ? atomic_dec_8_nv ((uint8_t  *)(v)) : \
>     sizeof(*v) == sizeof(uint16_t) ? atomic_dec_16_nv((uint16_t *)(v)) : \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_dec_32_nv((uint32_t *)(v)) : \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_dec_64_nv((uint64_t *)(v)) : \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_add(v, i) ((void)                                \
>     sizeof(*v) == sizeof(uint8_t)  ? atomic_add_8 ((uint8_t  *)(v), (i)) : \
>     sizeof(*v) == sizeof(uint16_t) ? atomic_add_16((uint16_t *)(v), (i)) : \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_add_32((uint32_t *)(v), (i)) : \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_add_64((uint64_t *)(v), (i)) : \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #define p_atomic_cmpxchg(v, old, _new) ((__typeof(*v)) \
>     sizeof(*v) == sizeof(uint8_t)  ? atomic_cas_8 ((uint8_t  *)(v), (uint8_t 
> )(old), (uint8_t )(_new)) : \
>     sizeof(*v) == sizeof(uint16_t) ? atomic_cas_16((uint16_t *)(v), 
> (uint16_t)(old), (uint16_t)(_new)) : \
>     sizeof(*v) == sizeof(uint32_t) ? atomic_cas_32((uint32_t *)(v), 
> (uint32_t)(old), (uint32_t)(_new)) : \
>     sizeof(*v) == sizeof(uint64_t) ? atomic_cas_64((uint64_t *)(v), 
> (uint64_t)(old), (uint64_t)(_new)) : \
> -                                    (assert(!"should not get here"), 0))
> +                                    (STATIC_ASSERT(!"should not get here"), 
> 0))
>  
>  #endif
>  
> -- 
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Attachment: signature.asc
Description: PGP signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to