Skip to content

Implement saturating_{add, sub} for non-native integer types #156

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

Merged
merged 5 commits into from
Apr 6, 2022

Conversation

yvt
Copy link
Contributor

@yvt yvt commented Apr 1, 2022

Fixes #155

Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this and improving the tests!

I'd like some changes in the code organization that should make this PR clearer, in my opinion.

@yvt
Copy link
Contributor Author

yvt commented Apr 5, 2022

Addressed the received feedback. Merged some commits with force-push to tidy up the history.

Updates their unsigned code paths to use the `Builder::gcc_` methods
that automatically lower non-native integer operations to native ones.

Also updates the signed code path of `saturating_add` to support non-
native integer types. That of `saturating_sub` already supports this,
so no major changes have been made.
@yvt yvt requested a review from antoyo April 6, 2022 04:08
Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Great job!
A few nits, though.

@yvt yvt requested a review from antoyo April 6, 2022 13:14
Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Just waiting for the CI to finish before merging.

@antoyo antoyo merged commit d69ada6 into rust-lang:master Apr 6, 2022
@yvt yvt deleted the fix-int-ops branch April 6, 2022 16:29
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.

Non-native u128 saturating ops causes ICE
3 participants