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

Float::ldexp should use the same format as everything else #22098

Closed
mdinger opened this issue Feb 8, 2015 · 6 comments · Fixed by #22631
Closed

Float::ldexp should use the same format as everything else #22098

mdinger opened this issue Feb 8, 2015 · 6 comments · Fixed by #22631
Assignees
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Milestone

Comments

@mdinger
Copy link
Contributor

mdinger commented Feb 8, 2015

std::num::Float::ldexp has an unusual format: it takes x: Self as it's first parameter where everything else in the trait takes self. It should take self as it's first parameter for consistency with everything else.

It is located in std::num::mod

@nagisa
Copy link
Member

nagisa commented Feb 10, 2015

+1. There are a few different things to consider here, though:

  1. ldexp does not operate on any FP number per se like most other operations do, but rather constructs a new FP number from two arguments one of which happens to be FP number. This function’s signature is consistent with most other methods that construct some kind of FP number (e.g. Float::zero()). We might want to consider whether methods such as next_after shouldn’t just go ahead and take x: Self instead.
  2. ldexp can’t currently be chained, but I see literally 0 cases where anybody would chain this function.

cc @huonw

@mdinger
Copy link
Contributor Author

mdinger commented Feb 10, 2015

Okay, but from a consistency standpoint, it's like 1/80 for x: self vs self. It it keeps the same format, it and it's similar functions would probably be better to go at the end or beginning of the function list where they won't be so unusual.

@aturon
Copy link
Member

aturon commented Feb 16, 2015

Nominating for 1.0-beta P-backcompat-libs

@pnkfelix
Copy link
Member

One should be able to call this thing as a method, one way or another. We need to make sure that is resolved.

@pnkfelix
Copy link
Member

1.0 beta, P-backcompat-libs

@pnkfelix pnkfelix added this to the 1.0 beta milestone Feb 19, 2015
@brson brson added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-nominated labels Feb 19, 2015
@milibopp
Copy link
Contributor

I started to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants