[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast
Erik Pilkington via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 24 14:47:28 PDT 2019
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 member}0"
----------------
Quuxplusone wrote:
> rsmith wrote:
> > "constexpr evaluate" doesn't really mean anything. Also, the "struct with a reference member type X" case seems a bit strangely phrased (and it need not be a struct anyway...).
> >
> > Maybe "cannot bit_cast {from|to}0 a {|type with a}1 {union|pointer|member pointer|volatile|reference}2 {type|member}1 in a constant expression"?
> Peanut gallery says: Surely wherever this message comes from should use the same wording as other similar messages: `"(this construction) is not allowed in a constant expression"`. That is, the diagnostics for
> ```
> constexpr int *foo(void *p) { return reinterpret_cast<int*>(p); }
> constexpr int *bar(void *p) { return std::bit_cast<int*>(p); }
> ```
> should be word-for-word identical except for the kind of cast they're complaining about.
>
> (And if I had my pedantic druthers, every such message would say "...does not produce a constant expression" instead of "...is not allowed in a constant expression." But that's way out of scope for this patch.)
Sure, that seems more consistent.
================
Comment at: clang/lib/AST/ExprConstant.cpp:5454-5457
+ case APValue::Indeterminate:
+ Info.FFDiag(BCE->getExprLoc(), diag::note_constexpr_access_uninit)
+ << 0 << 1;
+ return false;
----------------
rsmith wrote:
> I've checked the intent on the reflectors, and we should drop both this and the `APValue::None` check here. Bits corresponding to uninitialized or out-of-lifetime subobjects should just be left uninitialized (exactly like padding bits).
>
> Example:
>
> ```
> struct X { char c; int n; };
> struct Y { char data[sizeof(X)]; };
> constexpr bool test() {
> X x = {1, 2};
> Y y = __builtin_bit_cast(Y, x); // OK, y.data[1] - y.data[3] are APValue::Indeterminate
> X z = __builtin_bit_cast(X, y); // OK, both members are initialized
> return x.c == z.c && x.n == z.n;
> }
> static_assert(test());
> ```
You mean:
```
- struct Y { char data[sizeof(X)]; };
+ struct Y { unsigned char data[sizeof(X)]; };
```
...right? I added this test to the constexpr-builtin-bit-cast.cpp (as well of some of your other cases from the reflector).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62825/new/
https://reviews.llvm.org/D62825
More information about the llvm-commits
mailing list