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

Warning in docs of num_traits::cast::AsPrimitive seems to be out-of-date #174

Closed
jesperdj opened this issue Jun 18, 2020 · 2 comments · Fixed by #184
Closed

Warning in docs of num_traits::cast::AsPrimitive seems to be out-of-date #174

jesperdj opened this issue Jun 18, 2020 · 2 comments · Fixed by #184

Comments

@jesperdj
Copy link

jesperdj commented Jun 18, 2020

The documentation of num_traits::cast::AsPrimitive warns about undefined behavior:

Currently, some uses of the as operator are not entirely safe. In particular, it is undefined behavior if:
...
Or a floating point value does not fit in another floating point type (#15536)

However, issue 15536 is closed and fixed, as far as I understand, and this is not undefined behavior anymore (in fact, it was never really undefined behavior; the LLVM docs described it as such, but those docs have been clarified).

It looks like the warning in the documentation of AsPrimitive is not accurate anymore.

@Enet4
Copy link
Contributor

Enet4 commented Jul 2, 2020

That is good news! Resolving this issue appears to be a simple as removing the "Safety" section.

@cuviper
Copy link
Member

cuviper commented Jul 2, 2020

The float as float issue was deemed safe -- we can remove that.

But up through Rust 1.44, float as int is still UB if it overflows. In Rust 1.45, that operation will instead be defined to saturate, so only then it will be safe. We should probably still clarify this situation in the AsPrimitive docs.

Semi-related, #162 started using to_int_unchecked() in cases where we've already checked the boundary conditions, to avoid any possible performance hit of the upcoming saturation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants