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

Bratseth/pack bits #32537

Merged
merged 21 commits into from
Oct 21, 2024
Merged

Bratseth/pack bits #32537

merged 21 commits into from
Oct 21, 2024

Conversation

bratseth
Copy link
Member

@bratseth bratseth commented Oct 7, 2024

@geirst please review.

Sorry for the many commits, this was a bit exploratory.

Now that we have contextual type resolving, I want to replace the earlier half-baked attempts at type resolving by it, but this diff is already large enough.

@bratseth bratseth requested a review from geirst October 9, 2024 15:24
@bratseth
Copy link
Member Author

No rush, but have you seen this @geirst ?

@bratseth
Copy link
Member Author

Take your time @geirst ... :-)

geirst
geirst previously approved these changes Oct 21, 2024
Copy link
Member

@geirst geirst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I am not sure if the new input / output type resolving in StatementExpression.doVerify() is tested by existing unit tests or if new tests should be added (especially error situations).

@bratseth
Copy link
Member Author

Thanks! Yes, more work is needed on this type resolving for sure. This is just the bare minimum to support both embed and pack_bits in the same statement.

@bratseth bratseth merged commit 65eb3ea into master Oct 21, 2024
2 of 3 checks passed
@bratseth bratseth deleted the bratseth/pack_bits branch October 21, 2024 20:13
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.

2 participants