Page MenuHomePhabricator

use modern type trait implementations when available
ClosedPublic

Authored by rsmith on Jun 18 2018, 12:13 PM.

Details

Summary

Teach libcxx to stop using various deprecated __has_* type traits, in favor of the ("modern", C++11 era) __is_* type traits.

This is mostly just a simplification, but fixes at least one bug: _Atomic T should be considered trivially-destructible, but is not considered to be POD by Clang, and __has_trivial_destructor is specified in the GCC documentation as returning false for non-POD non-class types.

Diff Detail

Repository
rCXX libc++

Event Timeline

rsmith created this revision.Jun 18 2018, 12:13 PM
rsmith edited the summary of this revision. (Show Details)Jun 18 2018, 12:15 PM
EricWF accepted this revision.Apr 24 2019, 4:50 PM

LGTM minus nits.

include/type_traits
3742

We don't support anything before GCC 4.9, so you can replace the GCC guard with || defined(_LIBCPP_COMPILER_GCC)

3763–3764

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

3766

At this point I believe #elif !defined(_LIBCPP_CXX03_LANG) should work.

test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp
119

You can use TEST_HAS_BUILTIN_IDENTIFIER defined by test_macros.h (https://github.com/llvm-mirror/libcxx/blob/master/test/support/test_macros.h#L58)

This revision is now accepted and ready to land.Apr 24 2019, 4:50 PM
rsmith marked an inline comment as done.Apr 24 2019, 5:11 PM
rsmith added inline comments.
include/type_traits
3742

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.

3763–3764

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.

3766

I'll do that as part of the GCC4.9 change.

This revision was automatically updated to reflect the committed changes.

This patch is OK to land as-is.

Doing the cleanup separately makes sense.

include/type_traits
3742

That sounds good to me.

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

3763–3764

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.

rsmith marked an inline comment as done.Apr 24 2019, 5:53 PM
rsmith added inline comments.
include/type_traits
3742

This test is broken with Xcode 9:

-- Testing: 53613 tests, 8 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90
FAIL: libc++ :: std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp (50845 of 53613)
******************** TEST 'libc++ :: std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp' FAILED ********************
Command: ['/Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++', '-o', '/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/Output/is_trivially_destructible.pass.cpp.o', '-x', 'c++', '/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp', '-c', '-D_LIBCPP_DISABLE_AVAILABILITY', '-v', '-arch', 'x86_64', '-mmacosx-version-min=10.12', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++1z', '-include', '/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/nasty_macros.hpp', '-nostdinc++', '-I/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/include', '-I/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/include/c++build', '-isysroot', '/Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk', '-I/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c']
Exit Code: 1
Standard Error:
--
Apple LLVM version 9.0.0 (clang-900.0.37)
Target: x86_64-apple-darwin16.7.0
Thread model: posix
InstalledDir: /Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
 "/Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" -cc1 -triple x86_64-apple-macosx10.12.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name is_trivially_destructible.pass.cpp -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim -fno-strict-return -masm-verbose -munwind-tables -target-cpu penryn -target-linker-version 302.3 -v -dwarf-column-info -debugger-tuning=lldb -coverage-notes-file /b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/Output/is_trivially_destructible.pass.cpp.gcno -nostdinc++ -resource-dir /Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.0.0 -include /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/nasty_macros.hpp -isysroot /Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk -D _LIBCPP_DISABLE_AVAILABILITY -I /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/include -I /b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/include/c++build -I /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support -D "LIBCXX_FILESYSTEM_STATIC_TEST_ROOT=\"/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/input.output/filesystems/Inputs/static_test_env\"" -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT=\"/b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/filesystem/Output/dynamic_env\"" -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER=\"/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support/filesystem_dynamic_test_helper.py\"" -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -stdlib=libc++ -Werror=thread-safety -Wall -Wextra -Werror -Wuser-defined-warnings -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -std=c++1z -fdeprecated-macro -fdebug-compilation-dir /b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop -ftemplate-depth 270 -ferror-limit 19 -fmessage-length 0 -stack-protector 1 -fblocks -fobjc-runtime=macosx-10.12.0 -fencode-extended-block-signature -fcxx-exceptions -fexceptions -fmax-type-align=16 -fdiagnostics-show-option -o /b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/Output/is_trivially_destructible.pass.cpp.o -x c++ /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp
clang -cc1 version 9.0.0 (clang-900.0.37) default target x86_64-apple-darwin16.7.0
ignoring nonexistent directory "/Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/local/include"
ignoring nonexistent directory "/Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/Library/Frameworks"
#include "..." search starts here:
#include <...> search starts here:
 /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/include
 /b/rr/tmp7bTZVi/w/src/third_party/llvm-bootstrap/projects/libcxx/include/c++build
 /b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/support
 /Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.0.0/include
 /Applications/Xcode9.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include
 /Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include
 /Applications/Xcode9.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks (framework directory)
End of search list.
/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp:24:5: error: static_assert failed ""
    static_assert( std::is_trivially_destructible<T>::value, "");
    ^              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/b/rr/tmp7bTZVi/w/src/third_party/llvm/projects/libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_destructible.pass.cpp:120:5: note: in instantiation of function template specialization 'test_is_trivially_destructible<_Atomic(int)>' requested here
    test_is_trivially_destructible<_Atomic int>();
    ^

Does this just need an // XFAIL: apple-clang-9 or is this unexpected?

This test is broken with Xcode 9:

[...]

Does this just need an // XFAIL: apple-clang-9 or is this unexpected?

This seems to have been fixed in LLVM Clang trunk since the 8 release, and AppleClang 9 just doesn't have that fix. So I think it's reasonable to XFAIL it.

Clang 8: https://wandbox.org/permlink/nung9OHLrvObZGjq
Clang 9 (trunk): https://wandbox.org/permlink/y8e3UUx6CDf8x7im

Done in r359907, thanks!