When casting an enumerate with a fixed bool type the casting should use an IntegralToBoolean instead of an IntegralCast.
Fixes PR47055: Incorrect codegen for enum with bool underlying type
Details
Details
Diff Detail
Diff Detail
Event Timeline
| clang/test/AST/ast-dump-enum-bool.cpp | ||
|---|---|---|
| 1 | Why a (pretty unreadable) json test? There is also make-ast-dump-check.sh to auto-generate an ast dump test. | |
| clang/test/AST/ast-dump-enum-bool.cpp | ||
|---|---|---|
| 1 | I'd also like to see some CodeGen tests for various fixed underlying types that ensures the conversion to the underlying type happens before the conversion to the enumeration. Also, at least one test showing this behaves properly with a C-style cast instead of a named cast. | |
| clang/test/AST/ast-dump-enum-bool.cpp | ||
|---|---|---|
| 1 | In addition to CodeGen tests, we should also test that constant evaluation does the right thing. | |
| clang/test/AST/ast-dump-enum-bool.cpp | ||
|---|---|---|
| 1 | @riccibruno Thanks for the hint! I wasn't aware of this tool. I've modified the AST test. @aaron.ballman I've added a C-style AST test and CodeGen tests. However I'm not sure what you expect to see in the CodeGen tests. As I expected the CodeGen doesn't know about the enum, but it just uses the underlying type of the enum. Please let me know if you want to see additional tests. | |
| clang/test/AST/ast-dump-enum-bool.cpp | ||
|---|---|---|
| 2 | I don't think we really need a dedicated AST test for this. Such tests create a maintenance burden, and they don't really capture what we care about here: that all non-zero values are correctly converted to the true value of the enumeration type. | |
| clang/test/CodeGen/enum-bool.cpp | ||
| 7–14 ↗ | (On Diff #284478) | Some general guidance for writing IR testcases:
|
| clang/test/SemaCXX/cxx2a-consteval.cpp | ||
| 597–609 ↗ | (On Diff #284478) | This DR test should go in test/CXX/drs/dr23xx.cpp, along with a suitable "magic comment" // dr2338: 12 to indicate this DR is implemented in Clang 12 onwards. (We have tooling that generates the clang.llvm.org/cxx_dr_status.html page from those magic comments.) I don't think this has anything to do with consteval; a more-reduced test should work just as well (eg, static_assert((int)(E)2 == 1, "");) and would allow us to test this in C++11 onwards, not only in C++20. |
| clang/test/AST/ast-dump-enum-bool.cpp | ||
|---|---|---|
| 2 | I'll remove the test. | |
| clang/test/CodeGen/enum-bool.cpp | ||
| 7–14 ↗ | (On Diff #284478) | Thanks for the information, I'll reduce the test case. |
| clang/test/SemaCXX/cxx2a-consteval.cpp | ||
| 597–609 ↗ | (On Diff #284478) | I thought the magic comment could be done in any test file. It'll move the test and add the comment. |
Why a (pretty unreadable) json test? There is also make-ast-dump-check.sh to auto-generate an ast dump test.