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

[Alp Toker]

On 02/01/2014 04:04, Chandler Carruth wrote:
> On Wed, Jan 1, 2014 at 10:28 PM, Alp Toker <alp at nuanti.com 
> <mailto: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
>>     <mailto: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.

Totally. That's why I said "Stopgap measure until we can just use 
static_assert()" in my commit message.

Nevertheless the fallback you suggest SGTM and I've added it to 
Compiler.h LLVM_STATIC_ASSERT() in r198292 so we can have it both ways.

As for porting the assert in question, leaving that to someone who's 
more familiar with the IR unit tests.

Alp.


>
>     |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
>
>

-- 
http://www.nuanti.com
the browser experts