[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

This revision was automatically updated to reflect the committed changes. Closed by commit rL364954: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast (authored by epilk, committed by ). Changed prior to commit: https://reviews.llvm.org/D62825?vs=206304=207596#toc Repository:

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LG with a few tweaks. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:228-230 +def note_constexpr_bit_cast_indet_dest : Note< + "indeterminate value can only

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:223 +def note_constexpr_bit_cast_invalid_type : Note< + "cannot constexpr evaluate a bit_cast with a " + "%select{union|pointer|member pointer|volatile|struct with a reference

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington updated this revision to Diff 206304. erik.pilkington marked 10 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62825/new/ https://reviews.llvm.org/D62825 Files:

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:223 +def note_constexpr_bit_cast_invalid_type : Note< + "cannot constexpr evaluate a bit_cast with a " + "%select{union|pointer|member pointer|volatile|struct with a reference

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:223 +def note_constexpr_bit_cast_invalid_type : Note< + "cannot constexpr evaluate a bit_cast with a " + "%select{union|pointer|member pointer|volatile|struct with a reference member}0"

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington updated this revision to Diff 205713. erik.pilkington marked 16 inline comments as done. erik.pilkington added a comment. Address review comments, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62825/new/ https://reviews.llvm.org/D62825 Files:

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5461-5469 +case APValue::LValue: { + LValue LVal; + LVal.setFrom(Info.Ctx, Val); + APValue RVal; + if (!handleLValueToRValueConversion(Info, BCE, Ty.withConst(), +

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542662 , @rjmccall wrote: > In D62825#1542639 , @rsmith wrote: > > > In my view, the mistake was specifying `nullptr_t` to have the same size > > and alignment as `void*`; it

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rjmccall added a comment. In D62825#1542639 , @rsmith wrote: > In D62825#1542597 , @rjmccall wrote: > > > In D62825#1542374 , @rsmith wrote: > > > > > In D62825#1542309

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542597 , @rjmccall wrote: > In D62825#1542374 , @rsmith wrote: > > > In D62825#1542309 , @rjmccall > > wrote: > > > > > In D62825#1542301

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5360 + // FIXME: Its possible under the C++ standard for 'char' to not be 8 bits, but + // we don't support a host or target where that is the case. Still, we should + // use a more generic type in case

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rjmccall added a comment. In D62825#1542374 , @rsmith wrote: > In D62825#1542309 , @rjmccall wrote: > > > In D62825#1542301 , @rsmith wrote: > > > > > In D62825#1542247

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington added a comment. In D62825#1528329 , @rsmith wrote: > Can we also use the same bit reader/writer implementation to generalize the > current implementation of `__builtin_memcpy` and friends? (I don't think we > can remove the existing

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington updated this revision to Diff 204609. erik.pilkington marked 46 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62825/new/ https://reviews.llvm.org/D62825 Files:

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added a comment. In D62825#1542377 , @rsmith wrote: > (You might argue that it's ridiculous to require that `nullptr_t` have the > same size and alignment as `void*` but not have the same storage > representation as a null `void*`. I'd agree, and

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. (You might argue that it's ridiculous to require that `nullptr_t` have the same size and alignment as `void*` but not have the same storage representation as a null `void*`. I'd agree, and I've raised this in committee before, but without success) Repository: rC

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542309 , @rjmccall wrote: > In D62825#1542301 , @rsmith wrote: > > > In D62825#1542247 , @rjmccall > > wrote: > > > > > In what sense is

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rjmccall added a comment. In D62825#1542301 , @rsmith wrote: > In D62825#1542247 , @rjmccall wrote: > > > In what sense is the bit-pattern of a null pointer indeterminate? > > > The problem is not null pointers,

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542247 , @rjmccall wrote: > In what sense is the bit-pattern of a null pointer indeterminate? The problem is not null pointers, it's `nullptr_t`, which is required to have the same size and alignment as `void*` but

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rjmccall added a comment. In what sense is the bit-pattern of a null pointer indeterminate? It's implementation-specified, but the compiler is certainly required to be able to produce that value during the constant initialization phase. If the language committee wants to make this

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; rsmith wrote: > jfb wrote: > > rsmith wrote: > > > This will permit bitcasts from `nullptr_t`, providing a zero value. I > > > think

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; jfb wrote: > rsmith wrote: > > This will permit bitcasts from `nullptr_t`, providing a zero value. I think > > that's wrong; the bit

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; rsmith wrote: > This will permit bitcasts from `nullptr_t`, providing a zero value. I think > that's wrong; the bit representation of

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. Can we also use the same bit reader/writer implementation to generalize the current implementation of `__builtin_memcpy` and friends? (I don't think we can remove the existing implementation, sadly, since we allow copying arrays of the same trivially-copyable type today

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9716 +def err_bit_cast_non_trivially_copyable : Error< + "__builtin_bit_cast %select{source|destination}0 type must be a trivially copyable">; +def err_bit_cast_type_size_mismatch : Error<

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, jfb, rjmccall. Herald added subscribers: llvm-commits, kristina, arphaman, dexonsmith, jkorous, hiraditya. Herald added projects: clang, LLVM. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0476r2.html