-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Union Initializer Fails During CIRGen #1131
Comments
Maybe we shouldn't go into clangir/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp Lines 410 to 416 in 3aed38c
I'd like to look into the details a few days later. |
cc @gitoleg |
Sent #1160 |
bcardosolopes
pushed a commit
that referenced
this issue
Nov 27, 2024
…1166) Close #1131 This is another solution to #1160 This patch revert #1007 and remain its test. The problem described in #1007 is workaround by skipping the check of equivalent of element types in arrays. We can't mock such checks simply by adding another attribute to `ConstStructAttr` since the types are aggregated. e.g., we have to handle the cases like `struct { union { ... } }` and `struct { struct { union { ... } } }` and so on. To make it, we have to introduce what I called "two type systems" in #1160. This is not very good giving it removes a reasonable check. But it might not be so problematic since the Sema part has already checked it. (Of course, we still need face the risks to introduce new bugs any way)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following code snippet fails when generating CIR, and that didn't happen until PR#1007 was merged.
It seems there is some extra padding even after unpacking the union, and there is an error when computing the size?
I tried to fix it myself, but then it fails again when lowering to LLVM, because
b
is marked as inactive, anda
is the "active" field, but when lowering to LLVM, the type converter uses the larger member of the union.cc: @bcardosolopes and @ChuanqiXu9 since you authored the PR)
The text was updated successfully, but these errors were encountered: