[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

This revision was automatically updated to reflect the committed changes. Closed by commit rG9523cf02c22a: [AST] Fix handling of long double and bool in __builtin_bit_cast (authored by erik.pilkington). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

rsmith accepted this revision. rsmith added a comment. This looks OK to me. I think we'll also need updates for `_ExtInt` now too -- we have more fundamental types with padding than only `long double` and `bool` now. (I continue to be displeased that we assume 8-bit `char` in `__builtin_bit_cas

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

tambre added a comment. For reference, I've been using this + D75960 in my employer's Clang build for almost two months. We have been using `std::bit_cast` in our codebase quite a bit and haven't encountered any issues with these two changes. It would be nice t

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

ldionne added a comment. Gentle ping. This is blocking the implementation of `std::bit_cast` in libc++. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington added a comment. Ping, @rsmith did you want to take another look at this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington updated this revision to Diff 253986. erik.pilkington marked 5 inline comments as done. erik.pilkington added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 Files: clang/include/clang/Basic/

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:6364-6371 +const llvm::fltSemantics &Semantics = Info.Ctx.getFloatTypeSemantics(Ty); +unsigned NumBits = APFloat::semanticsSizeInBits(Semantics); +assert(NumBits % 8 == 0); +CharUnit

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:6364-6371 +const llvm::fltSemantics &Semantics = Info.Ctx.getFloatTypeSemantics(Ty); +unsigned NumBits = APFloat::semanticsSizeInBits(Semantics); +assert(NumBits % 8 == 0); +CharUnits Width =

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:6365 +const llvm::fltSemantics &Semantics = Info.Ctx.getFloatTypeSemantics(Ty); +unsigned NumBits = APFloat::semanticsSizeInBits(Semantics); +assert(NumBits % 8 == 0); l

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington updated this revision to Diff 252023. erik.pilkington marked 5 inline comments as done. erik.pilkington added a comment. Add tests for `bit_cast((char)1)`; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76323/new/ https://reviews.llvm.org/D76323 Files: clang/include/cl

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

jfb added a comment. Maybe you should test `nullptr` as well, given that it's all padding? Comment at: clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp:407 + +constexpr bool test_bad_bool = bit_cast('A'); // expected-error {{must be initialized by a constant expression}} expe

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM at high level, but I'm not really familiar with the code. Comment at: clang/lib/AST/ExprConstant.cpp:6365 +const llvm::fltSemantics &Semantics = Info.Ctx.getFloatT

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

erik.pilkington created this revision. erik.pilkington added reviewers: rsmith, ldionne, jfb. Herald added subscribers: ributzka, dexonsmith, jkorous. On x86, `long double` has 6 unused trailing bytes. This patch changes the constant evaluator to treat them as though they were padding bytes, so r