[CIR] Remove InavalidUnionFieldAttr and remove a check in array attr #1166
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChuanqiXu9
force-pushed
the
UnionRegression2
branch
from
4547582 to
c4ad031
Compare
bcardosolopes
approved these changes
Nov 27, 2024
| @@ -328,7 +328,8 @@ cir.func @cast27(%p : !u64i) { | |||
| !u32i = !cir.int<u, 32> | |||
| !u8i = !cir.int<u, 8> | |||
| module { | |||
| // expected-error@+1 {{constant array element should match array element type}} | |||
| // FIXME: The check for equality of the array element type is currently disabled due to https://github.com/llvm/clangir/pull/1007 | |||
There was a problem hiding this comment.
let's file an issue for this
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ConstStructAttrsince the types are aggregated. e.g., we have to handle the cases likestruct { union { ... } }andstruct { 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)