Move several float tests to floats/mod.rs #143738

rocurley · 2025-07-10T16:36:11Z

This PR moves several tests to floats/mod.rs, as discussed in #141726. The tests moved are:

test_num_f*
test_infinity
test_neg_infinity
test_zero
test_neg_zero
test_one
test_is_nan
test_is_infinite
test_is_finite
test_is_normal
test_classify

Each test is its own commit, so it may be easiest to review each commit individually.

r? tgross35

rocurley · 2025-07-10T16:37:46Z

library/coretests/tests/floats/mod.rs

    const LIM: Self = 1e-9;
 }

+trait TestableFloat {


Possibly we should merge Approx into this?

Alternatively, there could be separate floats for each logical grouping of constants. I don't really like that because it seems like a lot of boilerplate, but it would mean the traits here more meaningful.

Moving LIM to TestableFloat sounds good to me 👍 Maybe rename it APPROX there.

tgross35 · 2025-07-10T19:06:18Z

library/coretests/tests/floats/mod.rs

    const LIM: Self = 1e-9;
 }

+trait TestableFloat {


Moving LIM to TestableFloat sounds good to me 👍 Maybe rename it APPROX there.

tgross35 · 2025-07-10T19:08:06Z

library/coretests/tests/floats/mod.rs

+    test<Float> {
+    let neg_zero: Float = -0.0;
+    assert!(0.0 == neg_zero);
+    assert_biteq!(-0.0, neg_zero);
+    assert!(!neg_zero.is_infinite());
+    assert!(neg_zero.is_finite());
+    assert!(!neg_zero.is_sign_positive());
+    assert!(neg_zero.is_sign_negative());
+    assert!(!neg_zero.is_nan());
+    assert!(!neg_zero.is_normal());
+    assert!(matches!(neg_zero.classify(), Fp::Zero));
+    }


Could you indent this to match?

tgross35 · 2025-07-11T00:11:38Z

library/coretests/tests/floats/mod.rs

+impl TestableFloat for f16 {
+    const SMALL_NORMAL: Self = 1e-4;
+    const SUBNORMAL: Self = 1e-5;
+}
+
+impl TestableFloat for f32 {
+    const SMALL_NORMAL: Self = 1e-37;
+    const SUBNORMAL: Self = 1e-38;
+}
+
+impl TestableFloat for f64 {
+    const SMALL_NORMAL: Self = 1e-307;
+    const SUBNORMAL: Self = 1e-308;
+}
+
+impl TestableFloat for f128 {
+    const SMALL_NORMAL: Self = 1e-4931;
+    const SUBNORMAL: Self = 1e-4932;
+}


Could you rename SMALL_NORMAL to MIN_POSITIVE_NORMAL? Then these can just be f32::MIN_POSITIVE rather than a handwritten value. You could also use MIN_POSITIVE directly, but that name is confusing.

SUBNORMAL can similarly be renamed to MAX_SUBNORMAL with a value f32::MIN_POSITIVE.next_down().

rocurley · 2025-07-11T02:11:14Z

@rustbot review

tgross35

Couple of other small things.

After this, please clean up history by squashing the fixup commits. Or just squash everything together, if that's easier.

tgross35 · 2025-07-11T02:52:45Z

library/coretests/tests/floats/mod.rs

+        assert!(!0.0f32.is_nan());
+        assert!(!5.3f32.is_nan());
+        assert!(!(-10.732f32).is_nan());


These are all f32 so you'll need to bind them to a variable of type Float

tgross35 · 2025-07-11T02:55:01Z

library/coretests/tests/floats/mod.rs

+        assert!(matches!(neg_inf.classify(), Fp::Infinite));
+        assert!(matches!(zero.classify(), Fp::Zero));
+        assert!(matches!(neg_zero.classify(), Fp::Zero));
+        assert!(matches!(1f32.classify(), Fp::Normal));


Same here, this isn't the right type.

If it makes things easier you could add ZERO, ONE, NEG_ZERO and NEG_ONE to the trait since those specific values are used in multiple functions.

rocurley · 2025-07-11T14:47:19Z

Oof, thanks for catching those. Should be fixed now, and I've squashed the fixes into the appropriate original commits.

It seems like keeping the commit history clean is important here. I'm curious about the motivation: are the PR commits not squashed when merging into master?

@rustbot review

tgross35 · 2025-07-14T05:55:00Z

It seems like keeping the commit history clean is important here. I'm curious about the motivation: are the PR commits not squashed when merging into master?

They are not, the option would be nice but unfortunately Bors doesn't support it. Feel free to squash this if you get the chance before Bors picks it up since the commits are pretty tiny. No worries if you don't get to it since at least the history is clean.

@bors r+

bors · 2025-07-14T05:55:03Z

📌 Commit 79769f2 has been approved by tgross35

It is now in the queue for this repository.

…=tgross35 Move several float tests to floats/mod.rs This PR moves several tests to `floats/mod.rs`, as discussed in rust-lang#141726. The tests moved are: - `test_num_f*` - `test_infinity` - `test_neg_infinity` - `test_zero` - `test_neg_zero` - `test_one` - `test_is_nan` - `test_is_infinite` - `test_is_finite` - `test_is_normal` - `test_classify` Each test is its own commit, so it may be easiest to review each commit individually. r? tgross35

Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup

…=tgross35 Move several float tests to floats/mod.rs This PR moves several tests to `floats/mod.rs`, as discussed in rust-lang#141726. The tests moved are: - `test_num_f*` - `test_infinity` - `test_neg_infinity` - `test_zero` - `test_neg_zero` - `test_one` - `test_is_nan` - `test_is_infinite` - `test_is_finite` - `test_is_normal` - `test_classify` Each test is its own commit, so it may be easiest to review each commit individually. r? tgross35

Rollup of 13 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143957 (tidy: check for invalid file names) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) - #143990 (Add LocalKey<Cell>::update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143738 (Move several float tests to floats/mod.rs) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143957 (tidy: check for invalid file names) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) - #143990 (Add LocalKey<Cell>::update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #143738 - rocurley:float_tests_refactor_2, r=tgross35 Move several float tests to floats/mod.rs This PR moves several tests to `floats/mod.rs`, as discussed in #141726. The tests moved are: - `test_num_f*` - `test_infinity` - `test_neg_infinity` - `test_zero` - `test_neg_zero` - `test_one` - `test_is_nan` - `test_is_infinite` - `test_is_finite` - `test_is_normal` - `test_classify` Each test is its own commit, so it may be easiest to review each commit individually. r? tgross35

Rollup of 12 pull requests Successful merges: - rust-lang/rust#142936 (rustdoc-json: Structured attributes) - rust-lang/rust#143355 (wrapping shift: remove first bitmask and table) - rust-lang/rust#143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - rust-lang/rust#143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - rust-lang/rust#143738 (Move several float tests to floats/mod.rs) - rust-lang/rust#143920 (Make more of codegen_llvm safe) - rust-lang/rust#143921 (Constify `Index` traits) - rust-lang/rust#143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - rust-lang/rust#143948 (Update mdbook to 0.4.52) - rust-lang/rust#143957 (tidy: check for invalid file names) - rust-lang/rust#143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) - rust-lang/rust#143990 (Add LocalKey<Cell>::update) r? `@ghost` `@rustbot` modify labels: rollup

…=tgross35 Move several float tests to floats/mod.rs This PR moves several tests to `floats/mod.rs`, as discussed in rust-lang#141726. The tests moved are: - `test_num_f*` - `test_infinity` - `test_neg_infinity` - `test_zero` - `test_neg_zero` - `test_one` - `test_is_nan` - `test_is_infinite` - `test_is_finite` - `test_is_normal` - `test_classify` Each test is its own commit, so it may be easiest to review each commit individually. r? tgross35

…eltardieu Rollup of 12 pull requests Successful merges: - rust-lang#142936 (rustdoc-json: Structured attributes) - rust-lang#143355 (wrapping shift: remove first bitmask and table) - rust-lang#143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - rust-lang#143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - rust-lang#143738 (Move several float tests to floats/mod.rs) - rust-lang#143920 (Make more of codegen_llvm safe) - rust-lang#143921 (Constify `Index` traits) - rust-lang#143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - rust-lang#143948 (Update mdbook to 0.4.52) - rust-lang#143957 (tidy: check for invalid file names) - rust-lang#143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) - rust-lang#143990 (Add LocalKey<Cell>::update) r? `@ghost` `@rustbot` modify labels: rollup

…=tgross35 Move several float tests to floats/mod.rs This PR moves several tests to `floats/mod.rs`, as discussed in rust-lang#141726. The tests moved are: - `test_num_f*` - `test_infinity` - `test_neg_infinity` - `test_zero` - `test_neg_zero` - `test_one` - `test_is_nan` - `test_is_infinite` - `test_is_finite` - `test_is_normal` - `test_classify` Each test is its own commit, so it may be easiest to review each commit individually. r? tgross35

rustbot assigned tgross35 Jul 10, 2025

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 10, 2025

rocurley commented Jul 10, 2025

View reviewed changes

rocurley marked this pull request as ready for review July 10, 2025 16:38

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2025

tgross35 requested changes Jul 11, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 11, 2025

tgross35 reviewed Jul 11, 2025

View reviewed changes

rocurley added 11 commits July 11, 2025 10:31

Consolidate infinity tests

cfb66e5

Consolidate test_num tests

c5e67b4

Consolidate neg_infinity tests

c2e6b39

Consolidate zero tests

0c01322

Consolidate negative zero tests

fc01eed

Consolidate one tests

868020e

Consolidate is_nan

1b8904c

Consolidate is_infinite tests

e3d8367

Consolidate is_finite tests

7dd2811

Consolidate is_normal tests

d2c1900

Consolidate classify tests

79769f2

rocurley force-pushed the float_tests_refactor_2 branch from 261da66 to 79769f2 Compare July 11, 2025 14:41

tgross35 approved these changes Jul 14, 2025

View reviewed changes

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 Jul 14, 2025

samueltardieu mentioned this pull request Jul 15, 2025

Rollup of 11 pull requests #143976

Closed

samueltardieu mentioned this pull request Jul 15, 2025

Rollup of 11 pull requests #143978

Closed

samueltardieu mentioned this pull request Jul 15, 2025

Rollup of 10 pull requests #143982

Closed

samueltardieu mentioned this pull request Jul 15, 2025

Rollup of 12 pull requests #143983

Closed

samueltardieu mentioned this pull request Jul 16, 2025

Rollup of 13 pull requests #144017

Closed

samueltardieu mentioned this pull request Jul 16, 2025

Rollup of 12 pull requests #144028

Merged

bors merged commit 096a66d into rust-lang:master Jul 16, 2025
11 checks passed

rustbot added this to the 1.90.0 milestone Jul 16, 2025

Uh oh!

Move several float tests to floats/mod.rs #143738

Move several float tests to floats/mod.rs #143738

Uh oh!

Conversation

rocurley commented Jul 10, 2025

Uh oh!

rocurley Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

rocurley commented Jul 11, 2025

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

tgross35 Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

rocurley commented Jul 11, 2025

Uh oh!

tgross35 commented Jul 14, 2025

Uh oh!

bors commented Jul 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rocurley Jul 10, 2025 •

edited

Loading