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

Unified machine word trait #1676

Closed
wants to merge 4 commits into from
Closed

Unified machine word trait #1676

wants to merge 4 commits into from

Conversation

Kile-Asmussen
Copy link

A proposal to unify functionality pertaining to the primitive types in a single trait, easing generic programming with primitive types.

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jul 14, 2016
@kennytm
Copy link
Member

kennytm commented Jul 14, 2016

This is the same as the Int trait, which was removed from rust-lang/rust#23549 when rust-lang/rust#23104 was introduced.

Meanwhile you could use num_traits::PrimInt as a substitute, if you don't need to use every method (e.g. saturating_add etc).

(One of the goals of #369 was to provide "Limited generic programming: being able to work generically over the natural classes of primitive numeric types that vary only by size." But then the remaining traits proposed in #369 were removed as well ¯\_(ツ)_/¯)

fn bitwise_and(self, Self) -> Self; // new
fn bitwise_not(self) -> Self; // new
fn bitwise_or(self, Self) -> Self; // new
fn bitwise_xor(self, Self) -> Self; // new
Copy link
Member

@kennytm kennytm Jul 14, 2016

Choose a reason for hiding this comment

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

Why not simply make the trait implement BitAnd, Not, BitOr and BitXor?

You could call a.bitand(b) if you don't want to write a & b.

Copy link
Member

Choose a reason for hiding this comment

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

Yep - same for the others.

@withoutboats
Copy link
Contributor

I think this trait should be proposed as part of a comprehensive numerics proposal to adds all traits for numerics. I don't think we can evaluate this trait without thinking about how it overlaps and interacts with potential traits like Integer.

@Kile-Asmussen
Copy link
Author

@withoutboats According to philosophies of numerical computing proposed by John L. Gustafson in his book The End of Error: Unum Computing, there is merit in separating algebraic properties of the idealized numeric datatypes, from the "fast, dirty and specific" machine instructions.

In particular, we want algebraic operations to obey algebraic laws, while we want machine instructions to do one particular job in the right situation. This is particularly egregious in floating point optimizations that might insert a fused multiply-add whenever you write addition and multiplication in the same expression. Problem with FMA is that it does not obey algebraic laws (yields different results than multiply followed by add, due to rounding) but provides a more accurate result overall.

The point of having a separate machine word trait is to separate concerns: the numeric hierarchy can focus on algebra, while this trait aims to be as close to metal as possible. If there was to be a MachineFloat trait, it could have FMA and Exact Doc Product and all the other nice hardware-level stuff for floats. This is an attempt at the same for machine integers.

@withoutboats
Copy link
Contributor

withoutboats commented Jul 15, 2016

@magnetohydrodynamics I don't disagree, I just think the whole system should be proposed concurrently, so we can discuss it holistically.

This RFC contains this proposal by its omission, which is hard to evaluate without seeing the other traits:

The point of having a separate machine word trait is to separate concerns: the numeric hierarchy can focus on algebra, while this trait aims to be as close to metal as possible.

- Make it easy to implement `i128`/`u128` (and larger types) when compiler/hardware support permits, letting
generic code predating the addition of the larger types work with it.
- Generic implementation of `std::ops::*` traits with the usual panic-on-overflow semantics, using the
`checked_*` functions.
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be more explicit about the benefits. What kind of code needs to work with generic overflow and what will be better about it? How does this trait make it easier to implement large integers in software? Why are generic ops traits desirable?

@brson
Copy link
Contributor

brson commented Jul 19, 2016

Neat proposal. Thanks for writing it up @magnetohydrodynamics!

Why does MachinArith depend on Copy + Eq? Can you make sure that's explicit in the RFC?

Do you have an implementation already? It might be good to prove this serves its purpose well in real generic code.

This is proposing a lot of new functions. It may be more appropriate and require less debate to consider the unification of existing features independently of adding new features.

The RFC should say where the trait will end up. core::num and std::num seem good.

[summary]: #summary

Unify functionality peculiar to `i8`…`i64` and `u8`…`u64` in a trait containing the
family of `overflowing`/`checked`/`wrapping`/`saturating` variants of arithmetic operations,
Copy link
Member

Choose a reason for hiding this comment

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

What about inheriting these from the existing traits instead of duplicating them?

Copy link
Member

@kennytm kennytm Jul 19, 2016

Choose a reason for hiding this comment

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

I don't think there are existing standard traits for the overflowing/checked/wrapping/saturating methods.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, woops! I must still be thinking in pre-1.0 land.

@aturon aturon self-assigned this Jul 22, 2016
@strega-nil
Copy link

ping @aturon @Karl-D-Asmussen

status?

@burdges
Copy link

burdges commented Dec 21, 2016

Appears min_value() and max_value() are missing from this, although probably they should be replaced by the associated constants MIN and MAX anyways.

@aturon
Copy link
Member

aturon commented Feb 1, 2017

Thanks much for the RFC. As some others have mentioned, numeric traits like this have had a long, sad history in the standard library. The strong preference of the libs team has been for both generic numeric programming, and a numeric hierarchy, to live outside of the standard library, at least at first. If such an external crate saw widespread use, we could imagine moving it into the standard library. But we've gotten it wrong too many times to start immediately with std. We even had something relatively close to this design, and ended up abandoning it.

As such, I'm going to propose closing this RFC for the time being, in favor of development in crates.io.

@rfcbot fcp close

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 1, 2017

Team member @aturon has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@aturon
Copy link
Member

aturon commented Feb 28, 2017

We discussed this RFC in libs triage, and in general there was consensus around the previous summary; we don't feel ready to revisit this question in the standard library, and would much prefer this to be developed out of tree. I'm going to close, in the meantime.

@aturon aturon closed this Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants