Skip to content
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

analyze: add support for some unsupported casts #929

Merged
merged 17 commits into from
Jun 8, 2023

Conversation

spernsteiner
Copy link
Collaborator

This adds various cases to emit_cast_desc_desc to eliminate all the "unsupported cast" warnings in the current tests, and also changes "unsupported cast" to a panic. In general, "unsupported cast" normally means we're generating code that won't compile (due to type errors), so this is actually a major error for the rewriter.

Currently based on #923; I will rebase onto master once that's been merged.

@spernsteiner spernsteiner force-pushed the analyze-unsupported-casts-base branch from 5949a50 to d1e05f3 Compare May 23, 2023 18:13
@spernsteiner spernsteiner force-pushed the analyze-unsupported-casts branch from a5f5566 to 086183a Compare May 23, 2023 18:13
@spernsteiner spernsteiner force-pushed the analyze-unsupported-casts branch from 086183a to 1fa5dd0 Compare June 7, 2023 22:34
@spernsteiner spernsteiner changed the base branch from analyze-unsupported-casts-base to master June 7, 2023 22:34
@spernsteiner spernsteiner requested a review from aneksteind June 7, 2023 22:39
@spernsteiner
Copy link
Collaborator Author

spernsteiner commented Jun 7, 2023

I rebased this onto master and ran into a test failure with the newly-added test function from #945. To fix it, I took several commits from #936 (all related to expanding mir_op's cast generation, so it makes sense to include them here) and added them to this branch. The commits in question are here.

I'm not sure if it's worth re-reviewing, since both this and #945 were already approved. @aneksteind, I've re-requested review just in case - feel free to just hit approve if you don't think it's necessary.

@kkysen
Copy link
Contributor

kkysen commented Jun 7, 2023

The new commits from #936 LGTM.

@aneksteind
Copy link
Contributor

@spernsteiner consider this re-approval 👍

@spernsteiner spernsteiner merged commit 6f1aeb8 into master Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants