[libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

[ldionne]

Summary

The macro was not defined in C++11 mode when it should have been, at least
according to how _LIBCPP_HAS_C11_FEATURES is defined.

[ldionne]

The goal of this commit is to fix build bot failures here: http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux-noexceptions/builds/286/steps/test.libcxx/logs/stdio. This failure was introduced when I fixed some unit tests that were no-ops in https://reviews.llvm.org/D50674 -- I didn't test that change on Linux so I didn't catch the failure before the bots notified me.

To test this change, I bootstrapped a Docker image with a configuration similar to the build bot, and was able to reproduce the issue. Applying this patch fixed the issue.

Side note: we should probably strive to have docker containers for as many libc++ build bots as possible -- this makes it possible to reproduce failures locally, which is otherwise a guessing game. Even better -- we could run better tests before submitting a change and avoid breaking the build altogether!

[jfb]

From __ISO_C_VISIBLE >= 2011 it looks like this tries to test C11 features regardless of the C++ version. That's probably fine, but we're walking this line where only C++17 really guarantees C11 support, so this entire thing is weird and brittle to me. You're not changing the brittle-ness, just removing one kink, so from that perspective this change LGTM.

libcxx/test/support/test_macros.h

127 ↗

(On Diff #160616)

Should this be TEST_STD_VER >= 11?

[mclow.lists]

I have pissed in this area, too - See https://bugs.llvm.org/show_bug.cgi?id=38495 and the proposed resolution here: https://bugs.llvm.org/attachment.cgi?id=20692
How about I just make this change as part of that fix?

[dexonsmith]

In D50719#1199427, @mclow.lists wrote:

I have pissed in this area, too - See https://bugs.llvm.org/show_bug.cgi?id=38495 and the proposed resolution here: https://bugs.llvm.org/attachment.cgi?id=20692
How about I just make this change as part of that fix?

If the bots are red from a previous commit I think it would be better for Louis to commit this separately.

[ldionne]

In D50719#1199427, @mclow.lists wrote:

I have pissed in this area, too - See https://bugs.llvm.org/show_bug.cgi?id=38495 and the proposed resolution here: https://bugs.llvm.org/attachment.cgi?id=20692
How about I just make this change as part of that fix?

Yes, that would be great.

[mclow.lists]

In D50719#1199450, @dexonsmith wrote:

If the bots are red from a previous commit I think it would be better for Louis to commit this separately.

I'm ok with that.

[ldionne]

Ok, I'm pushing with JF's suggested change (use TEST_STD_VER >= 11 instead of __cplusplus >= 201103L). Let's cross fingers that this is going to unbreak the testers -- like I said it fixed my Docker container.

[mclow.lists]

thinking about this more, I think this the wrong long-term direction.

The presence/absence of C11 features in the underlying C library does not depend on what version of the C++ language we're compiling with.
The tests that use TEXT_HAS_C11_XXX should be updated to check TEST_STD_VER instead.

However, I'm fine with this as a short-term fix.

[mclow.lists]

I will clean this up as part of https://bugs.llvm.org/show_bug.cgi?id=38495

test/support/test_macros.h

 #define TEST_NOEXCEPT throw()
 #define TEST_NOEXCEPT_FALSE
 #define TEST_NOEXCEPT_COND(...)
 #define TEST_THROW_SPEC(...) throw(__VA_ARGS__)
 #endif
 
 // Sniff out to see if the underling C library has C11 features
 // Note that at this time (July 2018), MacOS X and iOS do NOT.
-#if __ISO_C_VISIBLE >= 2011
+#if __ISO_C_VISIBLE >= 2011 || TEST_STD_VER >= 11
 #  if defined(__FreeBSD__)
 #    define TEST_HAS_C11_FEATURES
 #  elif defined(__Fuchsia__)
 #    define TEST_HAS_C11_FEATURES
 #  elif defined(__linux__)
 #    if !defined(_LIBCPP_HAS_MUSL_LIBC)
 #      if _LIBCPP_GLIBC_PREREQ(2, 17)
 #        define TEST_HAS_C11_FEATURES