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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits