Skip to content

Change overflowing_(add|sub|mul) to be non generic #29292

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

Closed
wants to merge 1 commit into from
Closed

Change overflowing_(add|sub|mul) to be non generic #29292

wants to merge 1 commit into from

Conversation

strega-nil
Copy link
Contributor

And also rename them to T_wrapping_(add|sub|mul), in order to allay confusion
with (add|sub|mul)_with_overflow.

And also rename them to T_wrapping_(add|sub|mul), in order to allay confusion
with (add|sub|mul)_with_overflow.
@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@hanna-kruppe
Copy link
Contributor

Wow, this is a pretty impressive diffstat. So large, in fact, that GitHub refuses to display a diff for libcore/num/mod.rs 😠 Could you please explain the motivation for this change and briefly summarize what all the diff noise in libcore/num/mod.rs is about?

@nagisa
Copy link
Member

nagisa commented Oct 25, 2015

Ditto #29288 (comment)

@killercup
Copy link
Member

You can see the full diff here, by the way: https://github.com/rust-lang/rust/pull/29292.patch

@strega-nil
Copy link
Contributor Author

@rkruppe It's not actually an impressive diff. It's a six line change, except that I need to stage0 that change inside a 500 line macro...

@strega-nil
Copy link
Contributor Author

If you have a better idea of how to implement it, I am all ears.

@strega-nil
Copy link
Contributor Author

I would also be happy to implement it like simd_add. I honestly didn't know that was an option.

@strega-nil strega-nil closed this Oct 26, 2015
@strega-nil strega-nil deleted the change-overflowing-intrinsics branch September 11, 2018 05:37
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.

7 participants