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

Pow(0, 0) no longer returnes 1 #78

Closed
wants to merge 3 commits into from
Closed

Pow(0, 0) no longer returnes 1 #78

wants to merge 3 commits into from

Conversation

meltinglava
Copy link

No description provided.

pow now panics in case of 0⁰
check_pow now returns None in case of 0⁰

pow and checked_pow now requires the trait 'Zero' as we need it to
check if base is zero when exp is zero.

Added basic seperate tests to check that basic behaivor works, and
that bought pow and checked_pow behaves correclty when 0⁰ is given.

Updated the documentation to state behavior in 0⁰ case.

Fixes #77
src/pow.rs Outdated
if exp == 0 {
return T::one();
return if base.is_zero() {
panic!("0⁰ is undefined")
Copy link
Member

Choose a reason for hiding this comment

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

I would restrain myself from using Unicode in panic messages as it can be problematic on platforms that do not support Unicode in CLI (aka Windows).

Copy link
Author

Choose a reason for hiding this comment

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

do we want:

  • 0^0
  • pow(0, 0)
  • any other?

Copy link
Member

Choose a reason for hiding this comment

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

I would go with pow(0, 0) as this is what user wrote. 0^0 is common syntax but in this case can be confused with XOR operator.

@@ -182,9 +183,13 @@ mod float_impls {
/// assert_eq!(pow(6u8, 3), 216);
/// ```
#[inline]
pub fn pow<T: Clone + One + Mul<T, Output = T>>(mut base: T, mut exp: usize) -> T {
pub fn pow<T: Clone + Zero + One + Mul<T, Output = T>>(mut base: T, mut exp: usize) -> T {
Copy link
Member

Choose a reason for hiding this comment

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

I believe that this is breaking change, so we should think if this is really only way.

@meltinglava
Copy link
Author

meltinglava commented Jul 18, 2018

Oddly rustc also prints 1 for 0⁰. Did not find a issue for it, nor anywhere that says that it should be like that. Did not even find a unit test for this case(in rustc). Might want to make an issue for rustc

@cuviper
Copy link
Member

cuviper commented Jul 18, 2018

cc #77 for the cross-reference

Many other languages return 1: https://rosettacode.org/wiki/Zero_to_the_zero_power

I think we should leave the current behavior alone and just document this edge case. Since we can't even check for this case without adding Zero in a breaking change, returning 1 is a reasonable compromise with tons of precedent.

@cuviper
Copy link
Member

cuviper commented Aug 6, 2018

Closing in favor of #79.

@cuviper cuviper closed this Aug 6, 2018
bors bot added a commit that referenced this pull request Aug 7, 2018
79: Updated documentation to note the pow(0, 0) case. r=cuviper a=meltinglava

Ref #78 

Co-authored-by: Roald <meltinglava>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants