[libcxx-commits] [PATCH] D48292: use modern type trait implementations when available

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.

This patch is OK to land as-is.

Doing the cleanup separately makes sense.



================
Comment at: include/type_traits:3683
+
+#elif __has_feature(has_trivial_destructor) || (_GNUC_VER >= 403)
+
----------------
rsmith wrote:
> EricWF wrote:
> > We don't support anything before GCC 4.9, so you can replace the GCC guard with `|| defined(_LIBCPP_COMPILER_GCC)`
> Nice. I'll do this more broadly as a separate patch, if that's OK with you. There's lots of support for older GCCs scattered around in this file and elsewhere.
That sounds good to me.

I'm happy to take over the cleanup if you want.


================
Comment at: include/type_traits:3710
 
-#if 0
+#ifndef _LIBCPP_HAS_NO_VARIADICS
+
----------------
rsmith wrote:
> EricWF wrote:
> > We should use variadics in C++03 when Clang is the compiler.  I would write this check as `#if !defined(_LIBCPP_CXX03_LANG) || defined(__clang__)`
> Would it make more sense to change the definition of _LIBCPP_HAS_NO_VARIADICS to do that globally? This same pattern is used in various places in libc++, both in existing code in `<type_traits>`, in `<future>` (for `packaged_task` / `async`), and in `<memory>` (for traits and `allocator::construct`). In every case we're guarding use of variadics to define a template that is specified as being variadic.
I'm trying to replace `_LIBCPP_HAS_NO_VARIADICS` with `_LIBCPP_CXX03_LANG`. Almost all usages of `_LIBCPP_HAS_NO_VARIADICS` guard more C++11 features than just variadic templates (like rvalue references) -- so simply changing the definition of `_LIBCPP_HAS_NO_VARIADICS` may be subtly wrong. 

I'll work on cleaning the macro up.








Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48292/new/

https://reviews.llvm.org/D48292