Restrict std::num::{Zero, One} implementors and improve std::num::pow implementation #11664

brendanzab · 2014-01-19T09:14:49Z

Zero and One have precise definitions in mathematics as the identities of the Add and Mul operations respectively. As such, types that implement these identities are now also required to implement their respective operator traits. This should reduce their misuse whilst still enabling them to be used in generalized algebraic structures (not just numbers). Existing usages of #[deriving(Zero)] in client code could break under these new rules, but this is probably a sign that they should have been using something like #[deriving(Default)] in the first place.

For more information regarding the mathematical definitions of the additive and multiplicative identities, see the following Wikipedia articles:

Note that for floating point numbers the laws specified in the doc comments of Zero::zero and One::one may not always hold. This is true however for many other traits currently implemented by floating point numbers. What traits floating point numbers should and should not implement is an open question that is beyond the scope of this pull request.

The implementation of std::num::pow has been made more succinct and no longer requires Clone. The coverage of the associated unit test has also been increased to test for more combinations of bases, exponents, and expected results.

huonw · 2014-01-19T13:00:46Z

I'm not keen on removing is_zero, since foo.is_zero() is cheaper than foo == zero() (for general numbers, e.g. testing each entry of a matrix for 0 is cheaper than creating a whole new zeroed matrix and comparing equality), and so algorithms like GCD should be using the "shortcut".

flaper87 · 2014-01-19T13:08:20Z

I'm not sure I agree with the is_zero removal. To be more precise, I don't like the fact that Float implements it and other numeric types don't. Although it's true that it hasn't been used quite much throughout Rust, it doesn't mean it is not useful for other cases.

I think we should keep it for now. Besides being convenient, it also removes the need of having Zero::zero() to verify is the value is zero.

brendanzab · 2014-01-19T13:12:31Z

Ok, all those reasons seem pretty valid. Should we add a One::is_one function too? One of the other reasons I removed it was that the asymmetry was really bugging me.

huonw · 2014-01-19T13:14:48Z

Maybe; but zero seems more fundamental, e.g. it's the base case of Euclid's algorithm, and exponentiation-by-squaring, etc etc.

brendanzab · 2014-01-19T13:16:53Z

Aren't all identities fundamental thingies though?

flaper87 · 2014-01-19T13:17:12Z

I don't think is_one is that necessary but I'm all for consistency. SO, +1 from me. I'd add it in a separate PR, though.

brendanzab · 2014-01-19T13:40:50Z

Rebased, rolling back the removal of Zero::is_zero.

brendanzab · 2014-01-19T13:49:35Z

Rebased - removed some gunk hanging around from the rollback.

flaper87 · 2014-01-19T14:07:40Z

small nit, otherwise LGTM

huonw · 2014-01-19T14:19:53Z

src/libstd/num/mod.rs

+    else {
+        let mut acc: T = one();
+        let mut base = base;
+        let mut exp = exp;


Both this and base can be mutable via mut base: T and mut exp: uint in the function definition.

Also, personally, I'd rather see if exp % 2 == 1 and exp /= 2 and just let the optimiser handle lowering it to optimised form.

Not sure if the optimizer will optimize it down to exp & 1 == 1, though. I recently proposed a patch to change this in the is_even implementation of some types.

It does. This is bread-and-butter peep-hole optimisation stuff. i.e. it actually gets "optimised" to have no modulus call even without any optimisations on, e.g.

#[crate_type="lib"]; #[no_mangle] pub fn parity(n: uint) -> bool { n % 2 == 1 }

Then compiling with rustc -S modulo.rs (no optimisations) and trimming out the relevant section of parity, gives:

andq $1, %rax # a bitwise and! cmpq $1, %rax sete %cl

(There's a pile of other junk from the possibility of % failing if the RHS is zero (this code path is removed with optimisation), and because of our poor handling of booleans.)

flaper87 · 2014-01-19T14:45:19Z

I'd prefer squashing these 2 commits into the other 2 if possible. No need to have them separate. The rest LGTM

brendanzab · 2014-01-19T14:55:43Z

@flaper87 Ok, squashed. Shame to see the comment history go, but no matter. More important is a clean git history!

flaper87 · 2014-01-19T14:57:06Z

LGTM, thanks a lot! @huonw r?

alexcrichton · 2014-01-19T18:34:51Z

src/libstd/num/mod.rs

+    ///
+    /// ~~~
+    /// a + 0 = x       ∀ a ∈ Self
+    /// 0 + a = x       ∀ a ∈ Self


Should these two laws have = a instead of = x?

Good catch!

alexcrichton · 2014-01-19T18:38:57Z

r=me with typo fix.

alexcrichton · 2014-01-19T18:56:28Z

Nice job on all this!

…their appropriate use Zero and One have precise definitions in mathematics. Documentation has been added to describe the appropriate uses for these traits and the laws that they should satisfy. For more information regarding these identities, see the following wikipedia pages: - http://wikipedia.org/wiki/Additive_identity - http://wikipedia.org/wiki/Multiplicative_identity

The implementation has been made more succinct and no longer requires Clone. The coverage of the associated unit test has also been increased to check more combinations of bases, exponents, and expected results.

brendanzab · 2014-01-20T07:16:33Z

@alexcrichton Rebased and did an r=you. Hopefully that's ok.

`Zero` and `One` have precise definitions in mathematics as the identities of the `Add` and `Mul` operations respectively. As such, types that implement these identities are now also required to implement their respective operator traits. This should reduce their misuse whilst still enabling them to be used in generalized algebraic structures (not just numbers). Existing usages of `#[deriving(Zero)]` in client code could break under these new rules, but this is probably a sign that they should have been using something like `#[deriving(Default)]` in the first place. For more information regarding the mathematical definitions of the additive and multiplicative identities, see the following Wikipedia articles: - http://wikipedia.org/wiki/Additive_identity - http://wikipedia.org/wiki/Multiplicative_identity Note that for floating point numbers the laws specified in the doc comments of `Zero::zero` and `One::one` may not always hold. This is true however for many other traits currently implemented by floating point numbers. What traits floating point numbers should and should not implement is an open question that is beyond the scope of this pull request. The implementation of `std::num::pow` has been made more succinct and no longer requires `Clone`. The coverage of the associated unit test has also been increased to test for more combinations of bases, exponents, and expected results.

Fix/11134 Fix rust-lang#11134 Hir of `qpath` will be `TypeRelative(Ty { kind: Path(LangItem...` when a closure contains macro (e.g. rust-lang/rust-clippy#11651) and rust-lang#11134, it causes panic. This PR avoids panicking and emitting incomplete path string when `qpath` contains `LangItem`. changelog: none

huonw reviewed Jan 19, 2014
View reviewed changes

alexcrichton reviewed Jan 19, 2014
View reviewed changes

brendanzab added 2 commits January 20, 2014 18:09

Improve std::num::pow implementation

509283d

The implementation has been made more succinct and no longer requires Clone. The coverage of the associated unit test has also been increased to check more combinations of bases, exponents, and expected results.

bors closed this Jan 20, 2014

bors merged commit 509283d into rust-lang:master Jan 20, 2014

brendanzab deleted the identities branch January 21, 2014 01:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restrict std::num::{Zero, One} implementors and improve std::num::pow implementation #11664

Restrict std::num::{Zero, One} implementors and improve std::num::pow implementation #11664

brendanzab commented Jan 19, 2014

huonw commented Jan 19, 2014

flaper87 commented Jan 19, 2014

brendanzab commented Jan 19, 2014

huonw commented Jan 19, 2014

brendanzab commented Jan 19, 2014

flaper87 commented Jan 19, 2014

brendanzab commented Jan 19, 2014

brendanzab commented Jan 19, 2014

flaper87 commented Jan 19, 2014

huonw Jan 19, 2014

flaper87 Jan 19, 2014

huonw Jan 19, 2014

flaper87 commented Jan 19, 2014

brendanzab commented Jan 19, 2014

flaper87 commented Jan 19, 2014

alexcrichton Jan 19, 2014

brendanzab Jan 19, 2014

alexcrichton commented Jan 19, 2014

alexcrichton commented Jan 19, 2014

brendanzab commented Jan 20, 2014

Restrict std::num::{Zero, One} implementors and improve std::num::pow implementation #11664

Restrict std::num::{Zero, One} implementors and improve std::num::pow implementation #11664

Conversation

brendanzab commented Jan 19, 2014

huonw commented Jan 19, 2014

flaper87 commented Jan 19, 2014

brendanzab commented Jan 19, 2014

huonw commented Jan 19, 2014

brendanzab commented Jan 19, 2014

flaper87 commented Jan 19, 2014

brendanzab commented Jan 19, 2014

brendanzab commented Jan 19, 2014

flaper87 commented Jan 19, 2014

huonw Jan 19, 2014

Choose a reason for hiding this comment

flaper87 Jan 19, 2014

Choose a reason for hiding this comment

huonw Jan 19, 2014

Choose a reason for hiding this comment

flaper87 commented Jan 19, 2014

brendanzab commented Jan 19, 2014

flaper87 commented Jan 19, 2014

alexcrichton Jan 19, 2014

Choose a reason for hiding this comment

brendanzab Jan 19, 2014

Choose a reason for hiding this comment

alexcrichton commented Jan 19, 2014

alexcrichton commented Jan 19, 2014

brendanzab commented Jan 20, 2014