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

Remove warning about f64->f32 cast being potential UB #49965

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Apr 14, 2018

As discussed in #15536, the LLVM documentation incorrect described overflowing f64->f32 casts as being undefined behavior. LLVM never treated them as such, and the documentation has been adjusted in https://reviews.llvm.org/rL329065. As such, this warning can now be removed.

Closes #49622.


I could not find any existing test checking for this warning. Should I be adding a test for the absence of the warning instead?

As discussed in rust-lang#15536, the LLVM documentation incorrect described
overflowing f64->f32 casts as being undefined behavior. LLVM never
treated them as such, and the documentation has been adjusted in
https://reviews.llvm.org/rL329065. As such, this warning can now
be removed.

Closes rust-lang#49622.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 14, 2018
@hanna-kruppe
Copy link
Contributor

Thanks! I don't think there's any value in testing for the absence of a diagnostic, so this is good to merge as-is.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 14, 2018

📌 Commit cbabb1b has been approved by rkruppe

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Apr 16, 2018
Remove warning about f64->f32 cast being potential UB

As discussed in rust-lang#15536, the LLVM documentation incorrect described overflowing f64->f32 casts as being undefined behavior. LLVM never treated them as such, and the documentation has been adjusted in https://reviews.llvm.org/rL329065. As such, this warning can now be removed.

Closes rust-lang#49622.

---

I could not find any existing test checking for this warning. Should I be adding a test for the absence of the warning instead?
bors added a commit that referenced this pull request Apr 16, 2018
Rollup of 8 pull requests

Successful merges:

 - #49555 (Inline most of the code paths for conversions with boxed slices)
 - #49606 (Prevent broken pipes causing ICEs)
 - #49646 (Use box syntax instead of Box::new in Mutex::remutex on Windows)
 - #49647 (Remove `underscore_lifetimes` and `match_default_bindings` from active feature list)
 - #49931 (Fix incorrect span in `&mut` suggestion)
 - #49959 (rustbuild: allow building tools with debuginfo)
 - #49965 (Remove warning about f64->f32 cast being potential UB)
 - #49994 (Remove unnecessary indentation in rustdoc book codeblock.)

Failed merges:
@bors bors merged commit cbabb1b into rust-lang:master Apr 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants