Repository navigation
NaN > NaN returns true #50811
Copy link
Copy link
Closed
Labels
A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)Area: Constant evaluation, covers all const contexts (static, const fn, ...)C-bugCategory: This is a bug.Category: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Description
The following tests do not pass for me, but I think they should, as comparisons with NaN should always simply evaluate to false:
#[test]
fn test_zero_zero() {
assert_eq!(0.0/0.0 < 0.0/0.0, false);
assert_eq!(0.0/0.0 > 0.0/0.0, false);
}
#[test]
fn test_nan_nan() {
assert_eq!(std::f64::NAN < std::f64::NAN, false);
assert_eq!(std::f64::NAN > std::f64::NAN, false);
}
fn main() {
println!("Hello, world!");
}
I get this result on both debug and release builds. I'm on an x86_64 running Linux, rustc 1.27.0-nightly (f0fdaba 2018-05-15).
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)Area: Constant evaluation, covers all const contexts (static, const fn, ...)C-bugCategory: This is a bug.Category: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Activity
jorendorff commentedon May 16, 2018
Playground link for this test.
https://play.rust-lang.org/?gist=4d6f7f54553b065615caac65b3cb6222&version=nightly&mode=debug
Affects both debug and release, Nightly and Stable.
varkor commentedon May 16, 2018
Playground:
I think this is an issue withassert_eq!rather than the floating-point comparisons, but I haven't dug into it yet.kennytm commentedon May 16, 2018
This seems to be a regression introduced by 1.26.
kennytm commentedon May 16, 2018
Reduced,
related to rvalue promotion I guess.Edit: Or maybe a bug in Miri (cc @oli-obk)
kennytm commentedon May 16, 2018
Bisection between c83fa5d...097efa9 gives #46882 as the regression PR, as expected.
oli-obk commentedon May 16, 2018
I guess this logic, which I blindly copied, was never correct to begin with:
https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/operator.rs#L188
I think this is how old const eval did it, and we didn't want to regress that. cc @eddyb
oli-obk commentedon May 16, 2018
40b118c#diff-580fb7d0b0ede57fb2ce24a5dd33910cL50
Has the comment about existing bad behaviour.
kennytm commentedon May 16, 2018
@oli-obk The old logic used unwrap, meaning
NaN > NaNwould just ICE (?). I think we can safely replace them directly asPrimVal::from_bool(l > r)etc.Edit: 🤔 The unwrap is in 40b118c#diff-e847a6da94440f7bf6783206efe4275bL138
kennytm commentedon May 16, 2018
The incorrect behavior mentioned in #50811 (comment) was this:
If we fix the float comparison behavior, this code will no longer compile. But I think this is clearly a bug fix and breaking code like this is acceptable.
nikomatsakis commentedon May 17, 2018
Fixed in #50812
Auto merge of #50812 - kennytm:fix-50756-miri-bad-float-behavior, r=o…
Fix issue rust-lang#50811 (`NaN > NaN` was true).
Auto merge of #51037 - pietroalbini:beta-backports, r=oli-obk
Auto merge of #51037 - pietroalbini:beta-backports, r=pietroalbini
Rollup merge of rust-lang#156317 - lokirithm:reorg-tests-04, r=Kivooeo
Auto merge of #156317 - lokirithm:reorg-tests-04, r=Kivooeo
Unrolled build for #156317