[ovs-dev] [PATCH] lib/unaligned.h - Fix C++ compiler issue with BUILD_ASSERT on Windows

Ben Pfaff blp at ovn.org
Wed Dec 20 22:35:26 UTC 2017


BUILD_ASSERT is meant to work everywhere.  By preference, please fix it.

On Wed, Dec 20, 2017 at 10:30:20PM +0000, Sairam Venugopal wrote:
> Hi Ben,
> 
> Sorry about not reading through the comments. If the assumption is to always support static_assert inside expressions for all languages and OS, then we can go ahead with the workaround prescribed in the post. However, if it’s not necessary, then we can skip supporting static_assert for those cases. Nevertheless, I think we are in agreement that this needs to be fixed in include/openvswitch/compiler.h and not lib/unaligned.h.
> 
> 
> Thanks,
> Sairam
> 
> On 12/20/17, 1:54 PM, "Ben Pfaff" <blp at ovn.org> wrote:
> 
> >The basic point here is that the BUILD_ASSERT macro is a build assertion
> >that works in an expression.  That is exactly what the comment says:
> >
> > * Use BUILD_ASSERT_DECL as a declaration or a statement, or BUILD_ASSERT as
> > * part of an expression. */
> >
> >Can you write a build assertion macro that works in an expression in
> >C++03 (I guess that's what you mean when you say C++?) and in C++11?
> >That is the goal.  I am sure that it can be done.  If it should be done
> >without static_assert for languages that do not support static_assert,
> >or do not support static_assert in an expression, then that's fine with
> >me.
> >
> >On Wed, Dec 20, 2017 at 09:34:16PM +0000, Sairam Venugopal wrote:
> >> Hi Ben,
> >> 
> >> That is not correct. Your suggestion would fix C++11 compilation on Linux and Windows. However,
> >> 
> >> it would still break on C++ on Windows. static_assert is a keyword on Windows on both C++ and C++11.
> >> However, this isn’t the case on Linux. It’s present only in C++11.
> >> 
> >> What is the inclination to use static_assert inside expressions on C++? Since this is an edge-case, I feel that
> >> we should isolate this behavior instead of letting it permeate. In future, if this is supported, it would be easier to fix this single nuance.
> >> 
> >> Eg: 
> >> Something like this:
> >> #define BUILD_ASSERT_IN_EXPR(EXPR) []{static_assert(EXPR, "assertion failed”);}
> >> 
> >> Thanks,
> >> Sairam
> >> 
> >> On 12/20/17, 8:18 AM, "Ben Pfaff" <blp at ovn.org> wrote:
> >> 
> >> >I don't know what you mean by "all existing definitions".  BUILD_ASSERT
> >> >has a few definitions but only one of them matters in this case:
> >> >
> >> >    #elif defined(__cplusplus) && __cplusplus >= 201103L
> >> >    #define BUILD_ASSERT(EXPR) static_assert(EXPR, "assertion failed")
> >> >
> >> >The post you cite implies that adding []{...} to the definition above
> >> >would fix the problem.  Is that correct?
> >> >
> >> >On Wed, Dec 20, 2017 at 12:45:04AM +0000, Sairam Venugopal wrote:
> >> >> Hi Ben,
> >> >> 
> >> >> I did consider updating the definition of BUILD_ASSERT, however, that would involve changing all existing definitions (even non-expression context).
> >> >> This issue is prevalent on both Linux and Windows when we start compiling in C++ 11.
> >> >> 
> >> >> Associated post:
> >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__stackoverflow.com_questions_31311748_is-2Dthere-2Dany-2Dway-2Dto-2Dslip-2Da-2Dstatic-2Dassert-2Dinto-2Dan-2Dexpression-2Din-2Diso-2Dc11&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=w5lx09Yyfn1993a2Tdgq4IXnaFsMjRcYiRSRftu2nek&s=6YRaA_irydpTc7N1-EXIctKiFclTsRSrh9XFKKRbQlA&e=
> >> >> 
> >> >> The ideal fix would be to not use static_assert as an expression in C++11 mode. 
> >> >> Currently, we hit this issue here : https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_blob_master_lib_unaligned.h-23L172&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=w5lx09Yyfn1993a2Tdgq4IXnaFsMjRcYiRSRftu2nek&s=X99J8FVdp3jAiS3mMS4DWKMbIJgphnTA5fptRDK_BXU&e=
> >> >> 
> >> >> You can disregard this patch for the time being since we will need to fix this on both Linux and Windows.
> >> >> 
> >> >> Thanks,
> >> >> Sairam
> >> >> 
> >> >> 
> >> >> 
> >> >> On 12/19/17, 3:45 PM, "Ben Pfaff" <blp at ovn.org> wrote:
> >> >> 
> >> >> >On Tue, Dec 19, 2017 at 03:32:27PM -0800, Sairam Venugopal wrote:
> >> >> >> A static assert declaration may appear at block scope (as a block
> >> >> >> declaration) and inside a class body (as a member declaration). However,
> >> >> >> unlike sizeof, static_assert cannot be used as an expression and this is
> >> >> >> strictly enforced on MSVC. error C2059: syntax error : static_assert
> >> >> >> 
> >> >> >> Signed-off-by: Sairam Venugopal <vsairam at vmware.com>
> >> >> >
> >> >> >I'd really prefer to adjust the definition of BUILD_ASSERT so that it
> >> >> >can be used in any expression context.  Did you consider that?


More information about the dev mailing list