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

transpile: allow stabilized const integer arithmetic methods #528

Merged
merged 1 commit into from
Jul 17, 2022

Conversation

afetisov
Copy link
Contributor

The wrapping arithmetic methods on unsigned integers are stably const now.

The wrapping arithmetic methods on unsigned integers are stably const now.
Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

This looks good, though untested. A translator test under tests/ would be nice.

Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

On second thought, I think this is a change we want regardless of if rustc can compile the resultant code. We shouldn't prematurely decide if it can or cannot, as rustc will likely give a much better error message anyways, and it's more future proof, as it allows rustc to improve separately.

@kkysen kkysen merged commit b7dd946 into immunant:master Jul 17, 2022
@afetisov afetisov deleted the stabilized-const-methods branch July 17, 2022 11:01
kkysen added a commit that referenced this pull request Jul 18, 2022
@kkysen
Copy link
Contributor

kkysen commented Jul 18, 2022

@afetisov, I just realized now, but this missed another such error on const wrapping arithmetic (fixed in 538690e) and the resulting unused ctx: ExprContext arguments (fixed in e3a3808). Hopefully we can get clippy in CI working soon so this will be caught.

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.

None yet

2 participants