f*::NAN: guarantee that this is a quiet NaN by RalfJung · Pull Request #139483 · rust-lang/rust (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
I think we should guarantee that this is a quiet NaN. This then implies that programs not using f*::from_bits (or unsafe type conversions) are guaranteed to only work with quiet NaNs. It would be awkward if people start to write 0.0 / 0.0 instead of using the constant just because they want to get a guaranteed-quiet NaN.
This is a @rust-lang/libs-api change. The definition of this constant currently is 0.0 / 0.0, which is already guaranteed to be a quiet NaN. So all this does is forward that guarantee to our users.
r? @thomcc
rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
Relevant to the library API team, which will review and decide on the PR/issue.
This change is insta-stable, so needs a completed FCP to proceed.
Status: PR is in FCP and is awaiting for FCP to complete.
and removed T-libs
Relevant to the library team, which will review and decide on the PR/issue.
Status: Awaiting review from the assignee but also interested parties.
Nominated for discussion during a libs-api team meeting.
labels
Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
The Miri subtree was changed
cc @rust-lang/miri
🔔 This is now entering its final comment period, as per the review above. 🔔
Is it worth considering the alternative of making no guarantees about f*::NAN, and instead providing f*::QUIET_NAN and f*::SIGNALING_NAN?
I think adding a SIGNALING_NAN might be worth considering, but I'd say NAN is the better name for QUIET_NAN since it's what you should use most of the time. Signaling NaNs show non-portable behavior much more quickly than quiet NaNs.
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
r? tgross35
@bors r+ rollup
📌 Commit 0da46d1 has been approved by tgross35
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
label
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#139351 (Autodiff batching2)
- rust-lang#139483 (f*::NAN: guarantee that this is a quiet NaN)
- rust-lang#139498 (Ignore zero-sized types in wasm future-compat warning)
- rust-lang#139967 (Introduce and use specialized
//@ ignore-auxiliaryfor test support files instead of using//@ ignore-test) - rust-lang#139969 (update libc)
- rust-lang#139971 (Make C string merging test work on MIPS)
- rust-lang#139974 (Change
InterpCx::instantiate*function visibility to pub) - rust-lang#139977 (Fix drop handling in
hint::select_unpredictable)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#139483 - RalfJung:nan, r=tgross35
f*::NAN: guarantee that this is a quiet NaN
I think we should guarantee that this is a quiet NaN. This then implies that programs not using f*::from_bits (or unsafe type conversions) are guaranteed to only work with quiet NaNs. It would be awkward if people start to write 0.0 / 0.0 instead of using the constant just because they want to get a guaranteed-quiet NaN.
This is a @rust-lang/libs-api change. The definition of this constant currently is 0.0 / 0.0, which is already guaranteed to be a quiet NaN. So all this does is forward that guarantee to our users.
I didn't think of this until just now but we should add a test ensuring the top mantissa bit is set
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
f*::NAN: guarantee that this is a quiet NaN
I think we should guarantee that this is a quiet NaN. This then implies that programs not using f*::from_bits (or unsafe type conversions) are guaranteed to only work with quiet NaNs. It would be awkward if people start to write 0.0 / 0.0 instead of using the constant just because they want to get a guaranteed-quiet NaN.
This is a @rust-lang/libs-api change. The definition of this constant currently is 0.0 / 0.0, which is already guaranteed to be a quiet NaN. So all this does is forward that guarantee to our users.
@tgross35 what's the best way to get the top mantissa bit for our various float types?
I think x.to_bits() & (1 << (F::MANTISSA_DIGITS - 2)) != 0 should work
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request
test_nan: ensure the NAN contant is quiet
Follow-up to rust-lang#139483
r? @tgross35
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request
test_nan: ensure the NAN contant is quiet
Follow-up to rust-lang#139483
r? @tgross35
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
test_nan: ensure the NAN contant is quiet
Follow-up to rust-lang#139483
r? @tgross35
Labels
This issue / PR is in PFCP or FCP with a disposition to merge it.
The final comment period is finished for this PR / Issue.
This change is insta-stable, so needs a completed FCP to proceed.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Status: PR is in FCP and is awaiting for FCP to complete.
Relevant to the library API team, which will review and decide on the PR/issue.