Page MenuHomePhabricator

[libc++] Implement C++20's P0476r2: std::bit_cast
Needs ReviewPublic

Authored by ldionne on Mar 10 2020, 1:43 PM.

Details

Reviewers
jfb
EricWF
mclow.lists
Group Reviewers
Restricted Project

Diff Detail

Unit TestsFailed

TimeTest
930 mslibc++.libcxx/diagnostics::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/dev/null', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/libcxx/diagnostics/nodiscard_extensions.fail.cpp', '-c', '-v', '-ftemplate-depth=270', '-fsyntax-only', '-Xclang', '-verify', '-Xclang', '-verify-ignore-unexpected=note', '-ferror-limit=1024', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/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', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-Wno-error=user-defined-warnings', '-c'] Exit Code: 1 Standard Error:
1,080 mslibc++.libcxx/thread/thread_lock/thread_lock_guard::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/dev/null', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.fail.cpp', '-c', '-v', '-ftemplate-depth=270', '-fsyntax-only', '-Xclang', '-verify', '-Xclang', '-verify-ignore-unexpected=note', '-ferror-limit=1024', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/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', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-Wno-error=user-defined-warnings', '-c'] Exit Code: 1 Standard Error:
1,180 mslibc++.std/numerics/bit/bit_cast::Unknown Unit Message ("")
Compiled With: '/usr/bin/clang++ -o /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/numerics/bit/bit.cast/Output/bit_cast.pass.cpp.o -x c++ /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp -c -v -ftemplate-depth=270 -Werror=thread-safety -std=c++2a -include /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h -nostdinc++ -I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include -I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support -DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env" -DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env" -DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/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 -Wno-noexcept-type -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -c && /usr/bin/clang++ -o /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/numerics/bit/bit.cast/Output/bit_cast.pass.cpp.exe /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/numerics/bit/bit.cast/Output/bit_cast.pass.cpp.o -v -ftemplate-depth=270 -L/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/./lib -Wl,-rpath…
220 mslibunwind.libunwind::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libunwind/test/Output/frameheadercache_test.pass.cpp.o', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libunwind/test/frameheadercache_test.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-DLIBUNWIND_NO_TIMER', '-funwind-tables', '-std=c++2a', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libunwind/include', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libunwind/../libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libunwind/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libunwind/../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', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c'] Exit Code: 1 Standard Error:

Event Timeline

ldionne created this revision.Mar 10 2020, 1:43 PM
Herald added a project: Restricted Project. · View Herald Transcript
EricWF requested changes to this revision.Mar 11 2020, 8:12 PM

LGTM other than the lack of tests.

We should also figure out what GCC's plan is here, so we can implement a GCC version in future.

libcxx/include/bit
366

We should probably mark this nodiscard.

371

Gosh. __builtin_bit_cast is a weird builtin -- taking a type as the first parameter.

373

#endif // !defined(_LIBCPP_HAS_NO_BUILTIN_BIT_CAST)

libcxx/test/std/numerics/bit/bit.cast/bit_cast.fail.cpp
59

Are these errors emitted inside <bit>? If so, we should specify that here.

libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp
25

What's the fun stuff?

This revision now requires changes to proceed.Mar 11 2020, 8:12 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 11 2020, 8:12 PM
ldionne updated this revision to Diff 250243.Mar 13 2020, 9:54 AM
ldionne marked 5 inline comments as done.

Add tests

ldionne updated this revision to Diff 250245.Mar 13 2020, 9:58 AM
ldionne marked 3 inline comments as done and an inline comment as not done.

Specify that the errors from __builtin_bit_cast come from <bit>

ldionne added inline comments.
libcxx/include/bit
366

Marked it with _LIBCPP_NODISCARD_EXT, which we use for nodiscard-as-an-extension.

368

Question: the spec has these as Constraints: -- I think it means these should be used to SFINAE out, not hard-error -- right?

libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp
25

Writing the tests.

154

@erik.pilkington Any idea why those don't work inside constexpr?

157

@jfb Can you think of other interesting values to test?

197

@erik.pilkington All the tests for long double are failing (at runtime) -- any idea why? Did you implement support for long double in __builtin_bit_cast?

ldionne edited the summary of this revision. (Show Details)Mar 13 2020, 9:59 AM
libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp
154

Its hitting the isInt() check here: https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ExprConstant.cpp#L11102. Seems like constexpr __builtin_memcmp was only implemented for integers.

197

Eek, it looks like this is asserting at compile time too. Let me take a closer look.

jfb added inline comments.Mar 13 2020, 10:52 AM
libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp
164

Signaling NaN, and NaNs with payloads.
Infinities.
Denormals.

Check this out: https://babbage.cs.qc.cuny.edu/IEEE-754/index.xhtml

194

Some of these won't work on all platforms. You'll need to selectively disable some for platforms that don't have IEEE-754 with the expected sizes.

libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp
197

I put a patch up to fix this here: https://reviews.llvm.org/D76323

tambre added a subscriber: tambre.Jun 20 2020, 4:19 AM
tambre added inline comments.Jun 20 2020, 4:29 AM
libcxx/include/bit
368

Correct.

From [structure.specifications] 3.1:

Constraints: the conditions for the function’s participation in overload resolution.