8

Add RFC float-next-up-down. by orlp · Pull Request #3173 · rust-lang/rfcs · GitH...

 3 years ago
source link: https://github.com/rust-lang/rfcs/pull/3173
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Copy link

orlp commented 12 days ago

edited

This RFC adds two argumentless methods to f32/f64, next_up and next_down. These functions are specified in the IEEE 754 standard, and provide the capability to enumerate floating point values in order.

Rendered.

There's also already an associated implementation pull request (rust-lang/rust#88728), should this RFC pass or be deemed unnecessary.

Copy link

Author

orlp commented 12 days ago

@kennytm I believe it falls under the 'new API' section, which suggests making an RFC.

Copy link

Contributor

clarfonthey commented 12 days ago

Whether new APIs require RFCs generally depends on surface area. Regardless, you've certainly put in the time to justify the change.

Copy link

leonardo-m commented 12 days ago

edited

I'm in favour of these two functions, for me their usage is rare but I've used them once or twice in other languages. The point of this RFC is to link to it from those function docs, to give complete documentation.

(Regarding the names: as long as the documentation contains the common C function names, I think that next_float and prev_float are simpler to understand for common humans. Rust stdlib generally prefers more self-explanatory function names instead of the common and often more cryptic names. And I agree with this because remembering tens of strange function names isn't nice).

Copy link

Contributor

@nagisa nagisa left a comment

Much like the others, I feel like API addition of this scope does not necessarily warrant a full RFC process. Since we do have a nicely written RFC, perhaps consider making a PR with an implementation in parallel to this RFC as well? I suspect that it would be merged and made available to nightly users significantly before the RFC process concludes.

Two more functions get added to `f32` and `f64`, which may be considered

already cluttered by some.

Additionally, there is a minor pitfall regarding signed zero. Repeatedly calling

Copy link

Contributor

@nagisa nagisa 12 days ago

This behaviour follows from the definition of the function, doesn't it? The definition says:

Returns the largest/smallest number more/less than self.

Since 0.0 and -0.0 compare equal, 0.0.next_down() == -0.0 would violate the contract.

Copy link

Author

@orlp orlp 12 days ago

Yes, if we didn't want this we would need to change the definition of the function, and depart from IEEE 754 compliance. But I still felt I needed to mention this potential pitfall.

then `x` is supposed to be returned. Besides error signaling and NaNs, that is

the complete specification.

We did not choose this function for three reasons:

Copy link

Contributor

@nagisa nagisa 12 days ago

next_up and next_down are also easier to implement more efficiently as they don't need to dispatch algorithm based on 2 inputs.

# Unresolved questions

[unresolved-questions]: #unresolved-questions

- Which is the better pair of names, `next_up` and `next_down` or `next_float`

Copy link

Contributor

@nagisa nagisa 12 days ago

_float is already implied by the type in Rust.

Copy link

Author

orlp commented 12 days ago

edited

Much like the others, I feel like API addition of this scope does not necessarily warrant a full RFC process.

Part of the reason I chose to come directly in full force is because next_after was part of Rust, but got deprecated. I felt I really had to make my case in a thought-out manner.

Since we do have a nicely written RFC, perhaps consider making a PR with an implementation in parallel to this RFC as well?

I could do that, I also have written tests, not part of the RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

nagisa

Assignees

No one assigned

Labels
None yet
Projects

None yet

Milestone

No milestone

Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK