Message ID | 20170316210441.7138-1-blp@ovn.org |
---|---|
State | Accepted |
Headers | show |
LGTM with one question below, Acked-by: Jarno Rajahalme <jarno@ovn.org> > On Mar 16, 2017, at 2:04 PM, Ben Pfaff <blp@ovn.org> wrote: > > Until now, the BUILD_ASSERT and BUILD_ASSERT_DECL macros have used OVS's > home-grown build assertion strategy. This commit switches them to using > C11 build assertions with compilers that support them. The semantics are > the same, but C11 build assertions yield clearer error messages when they > fail. > > This commit also reorders the definitions a bit to make it easier to > follow. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > include/openvswitch/compiler.h | 28 +++++++++++++++------------- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h > index 6e779f38fd16..0dc8636add33 100644 > --- a/include/openvswitch/compiler.h > +++ b/include/openvswitch/compiler.h > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016, 2017 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -236,26 +236,28 @@ > #define OVS_PREFETCH_WRITE(addr) > #endif > > -/* Build assertions. */ > +/* Build assertions. > + * > + * Use BUILD_ASSERT_DECL as a declaration or a statement, or BUILD_ASSERT as > + * part of an expression. */ > #ifdef __CHECKER__ > #define BUILD_ASSERT(EXPR) ((void) 0) > #define BUILD_ASSERT_DECL(EXPR) extern int (*build_assert(void))[1] > -#elif !defined(__cplusplus) > -/* Build-time assertion building block. */ > +#elif defined(__cplusplus) > +#include <boost/static_assert.hpp> > +#define BUILD_ASSERT BOOST_STATIC_ASSERT > +#define BUILD_ASSERT_DECL BOOST_STATIC_ASSERT > +#elif (__GNUC__ * 256 + __GNUC_MINOR__ >= 0x403 \ > + || __has_extension(c_static_assert)) > +#define BUILD_ASSERT_DECL(EXPR) _Static_assert(EXPR, #EXPR) > +#define BUILD_ASSERT(EXPR) (void) ({ _Static_assert(EXPR, #EXPR); }) Curly braces in a macro is a GCC feature, so is it possible that a compiler has the “c_static_assert” extension but not this one? I see that __has_extension() is defined as 0 if it is not defined, so if it it only ever defined for GCC or compatible compiler, then this question is moot. Jarno > +#else > #define BUILD_ASSERT__(EXPR) \ > sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1; }) > - > -/* Build-time assertion for use in a statement context. */ > #define BUILD_ASSERT(EXPR) (void) BUILD_ASSERT__(EXPR) > - > -/* Build-time assertion for use in a declaration context. */ > #define BUILD_ASSERT_DECL(EXPR) \ > extern int (*build_assert(void))[BUILD_ASSERT__(EXPR)] > -#else /* __cplusplus */ > -#include <boost/static_assert.hpp> > -#define BUILD_ASSERT BOOST_STATIC_ASSERT > -#define BUILD_ASSERT_DECL BOOST_STATIC_ASSERT > -#endif /* __cplusplus */ > +#endif > > #ifdef __GNUC__ > #define BUILD_ASSERT_GCCONLY(EXPR) BUILD_ASSERT(EXPR) > -- > 2.10.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Mar 16, 2017 at 05:24:59PM -0700, Jarno Rajahalme wrote: > LGTM with one question below, > > Acked-by: Jarno Rajahalme <jarno@ovn.org> Thanks! > > On Mar 16, 2017, at 2:04 PM, Ben Pfaff <blp@ovn.org> wrote: > > +#elif (__GNUC__ * 256 + __GNUC_MINOR__ >= 0x403 \ > > + || __has_extension(c_static_assert)) > > +#define BUILD_ASSERT_DECL(EXPR) _Static_assert(EXPR, #EXPR) > > +#define BUILD_ASSERT(EXPR) (void) ({ _Static_assert(EXPR, #EXPR); }) > > Curly braces in a macro is a GCC feature, so is it possible that a > compiler has the “c_static_assert” extension but not this one? I see > that __has_extension() is defined as 0 if it is not defined, so if it > it only ever defined for GCC or compatible compiler, then this > question is moot. To the best of my knowledge, only Clang implements __has_extension. OVS only really supports GCC, Clang, and MSVC, although adding support for other compilers is probably not too hard if they're decent compilers. So I think we're probably OK. I applied this to master.
diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h index 6e779f38fd16..0dc8636add33 100644 --- a/include/openvswitch/compiler.h +++ b/include/openvswitch/compiler.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016, 2017 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -236,26 +236,28 @@ #define OVS_PREFETCH_WRITE(addr) #endif -/* Build assertions. */ +/* Build assertions. + * + * Use BUILD_ASSERT_DECL as a declaration or a statement, or BUILD_ASSERT as + * part of an expression. */ #ifdef __CHECKER__ #define BUILD_ASSERT(EXPR) ((void) 0) #define BUILD_ASSERT_DECL(EXPR) extern int (*build_assert(void))[1] -#elif !defined(__cplusplus) -/* Build-time assertion building block. */ +#elif defined(__cplusplus) +#include <boost/static_assert.hpp> +#define BUILD_ASSERT BOOST_STATIC_ASSERT +#define BUILD_ASSERT_DECL BOOST_STATIC_ASSERT +#elif (__GNUC__ * 256 + __GNUC_MINOR__ >= 0x403 \ + || __has_extension(c_static_assert)) +#define BUILD_ASSERT_DECL(EXPR) _Static_assert(EXPR, #EXPR) +#define BUILD_ASSERT(EXPR) (void) ({ _Static_assert(EXPR, #EXPR); }) +#else #define BUILD_ASSERT__(EXPR) \ sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1; }) - -/* Build-time assertion for use in a statement context. */ #define BUILD_ASSERT(EXPR) (void) BUILD_ASSERT__(EXPR) - -/* Build-time assertion for use in a declaration context. */ #define BUILD_ASSERT_DECL(EXPR) \ extern int (*build_assert(void))[BUILD_ASSERT__(EXPR)] -#else /* __cplusplus */ -#include <boost/static_assert.hpp> -#define BUILD_ASSERT BOOST_STATIC_ASSERT -#define BUILD_ASSERT_DECL BOOST_STATIC_ASSERT -#endif /* __cplusplus */ +#endif #ifdef __GNUC__ #define BUILD_ASSERT_GCCONLY(EXPR) BUILD_ASSERT(EXPR)
Until now, the BUILD_ASSERT and BUILD_ASSERT_DECL macros have used OVS's home-grown build assertion strategy. This commit switches them to using C11 build assertions with compilers that support them. The semantics are the same, but C11 build assertions yield clearer error messages when they fail. This commit also reorders the definitions a bit to make it easier to follow. Signed-off-by: Ben Pfaff <blp@ovn.org> --- include/openvswitch/compiler.h | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)