[llvm] r198273 - Silence g++ 4.9 build issue in unit tests

[Chandler Carruth]

On Wed, Jan 1, 2014 at 10:28 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 02/01/2014 02:52, Chandler Carruth wrote:
>
>
> On Wed, Jan 1, 2014 at 5:35 PM, Alp Toker <alp at nuanti.com> wrote:
>
>>  & is there a reason we're not using the LLVM_STATIC_ASSERT macro here?
>>>
>>
>>  Requires LLVM_ENABLE_CXX11 which isn't yet enabled on most (any?) of the
>> builders.
>>
>> I'd just wait until C++11 and change the lot.
>
>
> Did you check the definition? We provide a fallback if the compiler
> doesn't support static_assert.
>
>
> Sure, in fact I wrote the current definition of LLVM_STATIC_ASSERT (or at
> least made the changes adding compiler checks in r198142 and r198255).
>
> There is no fallback..
>
> /// \macro LLVM_STATIC_ASSERT
> /// \brief Expands to C/C++'s static_assert on compilers which support it.
> #if __has_feature(cxx_static_assert) || \
>     defined(__GXX_EXPERIMENTAL_CXX0X__) || LLVM_MSC_PREREQ(1600)
> # define LLVM_STATIC_ASSERT(expr, msg) static_assert(expr, msg)
> #elif __has_feature(c_static_assert)
> # define LLVM_STATIC_ASSERT(expr, msg) _Static_assert(expr, msg)
> #else
> # define LLVM_STATIC_ASSERT(expr, msg)
> #endif
>
> The macro expands to nothing in the default build configuration:
>

That is the fallback to which I refer. What I'm saying is that you can
directly use LLVM_STATIC_ASSERT, you don't need to guard uses of it around
anything to do with C++11. And that seems fine, especially in a unit test.

I did misread it on the first pass -- it tests for the feature
static_assert, but not the extension static_assert, but that (and making it
actually useful with GCC) is an orthogonal discussion.

My real point was that I agreed with Dave here -- it seems it would be much
better to use LLVM_STATIC_ASSERT in this test rather than rolling our own
C++98 variant. If we want to make the LLVM_STATIC_ASSERT better so that it
fires in more build configuraitons, awesome, but I don't think its current
weakness is a good argument for not using it.


>
> lib/Lex/PPMacroExpansion.cpp:           .Case("cxx_static_assert",
> LangOpts.CPlusPlus11)
>
>
>   We also test for support of static_assert in a more fine grain way than
> merely that C++11 is enabled which should allow the macro to work on any
> build using a modern (last 2 years) Clang host compiler.
>
>
> Where?
>
> grep 'static_assert' through the LLVM source tree only finds the single
> line above.
>
> Alp.
>
>
>
>  -Chandler
>
>
> -- http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140101/a251ec27/attachment.html>