Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s #126636

tgross35 · 2024-06-18T17:17:38Z

This was originally a PR against the Clippy repo, rust-lang/rust-clippy#12950

r? @flip1995

Tracking issue: #116909
Fixes: #126636

rustbot · 2024-06-18T17:17:48Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

tgross35 · 2024-06-18T17:24:01Z

I know you suggested to include this as part of #126608, but it may as well just get its own PR since it touches a lot of files.

~~I need some help understanding https://github.com/rust-lang/rust-clippy/actions/runs/9569119207/job/26380925098?pr=12950#step:5:1055, I am not sure why it looks like it just deletes the file.~~ I just forgot to add the feature gates

@rustbot label +F-f16_and_f128

tgross35 · 2024-06-18T17:40:17Z

src/tools/clippy/clippy_utils/src/consts.rs

+            Self::F16(f) => {
+                f64::from(f).to_bits().hash(state);
+            },
            Self::F32(f) => {
                f64::from(f).to_bits().hash(state);
            },
            Self::F64(f) => {
                f.to_bits().hash(state);
            },
+            Self::F128(f) => {
+                f.to_bits().hash(state);
+            },


This part could still use a double check, not sure if there is something better to do here

I'm not sure why we go from f32 to f64 before hashing. But keeping this consistent for f16 seems reasonable 👍

I assume it is to make sure there is a hash that depends only on the value and not the size, which , but I can't find anywhere that it is being used.

Unfortunately, I forgot that conversions to/from f16 aren't yet present on all platforms so I had to change this to just f.to_bits().hash(state);. I left a fixme here instead, maybe we could convert everything to f128 once that is better supported.

maybe we could convert everything to f128 once that is better supported.

Might be a good idea, yes. But I wonder how much that matters of if we should just use f.to_bits().hash(state); everywhere. But that's not for this PR to decide. Let's leave the FIXME here and address this later.

flip1995 · 2024-06-19T09:18:48Z

src/tools/clippy/tests/ui/cast.rs

@@ -17,6 +17,8 @@
    clippy::identity_op
 )]

+// FIXME(f16_f128): add tests once const casting is available for these types


Can those tests be added with #126608 being merged? If so, after merging this PR, please add those in #126608

#126608 won't add casting, but it unblocks a followup PR that does add it. I'll make sure to add the Clippy tests with that one.

src/tools/clippy/tests/ui/cast_size.rs

This removes the ICE codepaths for `f16` and `f128` in Clippy. `rustc_apfloat` is used as a dependency for the parsing of these types, since their `FromStr` implementation will not be available in the standard library for a while.

tgross35 · 2024-06-19T17:42:55Z

I added a couple more FIXMEs in places where Constant::{F32, F64} is checked nonexhaustively but can't yet be added because it relies on math. @flip1995 to make things a bit more clear, here is my loose plan:

Merge Add more constants, functions, and tests for f16 and f128 #126608, which is blocked by this PR
Create a new PR along the lines of Add f16 and f128 const eval for binary and unary operationations #126429 that does const casting. This is blocked on Add more constants, functions, and tests for f16 and f128 #126608 and maybe also Update compiler_builtins to 0.1.114 #125016
Fix the FIXMEs here that depend on const casting, which is most of the tests
Once Update compiler_builtins to 0.1.114 #125016 merges it will unblock fixing everything that relies on non-const math. Even simple equality checks aren't available on all platforms without this.

flip1995 · 2024-06-20T11:45:28Z

Thanks for the summary what the current road map for this feature is!

Let's get this in to unblock you.

@bors r+

bors · 2024-06-20T11:45:31Z

📌 Commit 477e9e8 has been approved by flip1995

It is now in the queue for this repository.

…flip1995 Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s This was originally a PR against the Clippy repo, rust-lang/rust-clippy#12950 r? `@flip1995` Tracking issue: rust-lang#116909 Fixes: rust-lang#126636

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#126380 (Add std Xtensa targets support) - rust-lang#126636 (Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s ) - rust-lang#126659 (More status-quo tests for the `#[coverage(..)]` attribute) - rust-lang#126711 (Make Option::as_[mut_]slice const) - rust-lang#126717 (Clean up some comments near `use` declarations) - rust-lang#126719 (Fix assertion failure for some `Expect` diagnostics.) - rust-lang#126730 (Add opaque type corner case test) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#126636 - tgross35:clippy-f16-f128-fixme, r=flip1995 Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s This was originally a PR against the Clippy repo, rust-lang/rust-clippy#12950 r? ``@flip1995`` Tracking issue: rust-lang#116909 Fixes: rust-lang#126636

tgross35 · 2024-06-20T18:28:12Z

Thank you for the review!

Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s This was originally a PR against the Clippy repo, rust-lang#12950 r? ``@flip1995`` Tracking issue: rust-lang/rust#116909 Fixes: rust-lang/rust#126636

Add `f16` and `f128` math functions WIP This will be pretty finicky among selection failures, the lowering bug, and lack of symbols. But might as well start somewhere. Checks will fail until rust-lang#126636 makes it to beta, which should be next week (July 19). try-job: aarch64-gnu

Add `f16` and `f128` math functions This adds intrinsics and math functions for `f16` and `f128` floating point types. Support is quite limited and some things are broken so tests don't run on many platforms, but this provides a starting point. Checks will fail until rust-lang#126636 makes it to beta, which should be next week (July 19). try-job: aarch64-gnu

rustbot assigned flip1995 Jun 18, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 18, 2024

rustbot added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Jun 18, 2024

tgross35 force-pushed the clippy-f16-f128-fixme branch from 64443c7 to 26ef017 Compare June 18, 2024 17:35

tgross35 commented Jun 18, 2024

View reviewed changes

This was referenced Jun 19, 2024

Add more constants, functions, and tests for f16 and f128 #126608

Merged

ICE: not implemented: f16_f128 #122587

Closed

flip1995 reviewed Jun 19, 2024

View reviewed changes

flip1995 mentioned this pull request Jun 19, 2024

Resolve f16 and f128 unimplemented!/FIXMEs rust-lang/rust-clippy#12950

Closed

tgross35 added 2 commits June 19, 2024 13:30

Resolve Clippy f16 and f128 unimplemented!/FIXMEs

ff9efea

This removes the ICE codepaths for `f16` and `f128` in Clippy. `rustc_apfloat` is used as a dependency for the parsing of these types, since their `FromStr` implementation will not be available in the standard library for a while.

Update float tests to include f16 and f128

477e9e8

tgross35 force-pushed the clippy-f16-f128-fixme branch from 26ef017 to 477e9e8 Compare June 19, 2024 17:30

tgross35 mentioned this pull request Jun 20, 2024

Refactor Miri float casting tests #126727

Closed

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 Jun 20, 2024

This was referenced Jun 20, 2024

Rollup of 7 pull requests #126734

Closed

Rollup of 7 pull requests #126736

Merged

bors closed this in d3f5e7b Jun 20, 2024

bors merged commit d3f5e7b into rust-lang:master Jun 20, 2024
6 checks passed

tgross35 deleted the clippy-f16-f128-fixme branch June 20, 2024 18:28

tgross35 mentioned this pull request Jun 22, 2024

Tracking Issue for f16 and f128 float types #116909

Open

84 tasks

tgross35 mentioned this pull request Jul 12, 2024

Add f16 and f128 math functions #127027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s #126636

Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s #126636

tgross35 commented Jun 18, 2024 •

edited

Loading

rustbot commented Jun 18, 2024

tgross35 commented Jun 18, 2024 •

edited

Loading

tgross35 Jun 18, 2024

flip1995 Jun 19, 2024

tgross35 Jun 19, 2024

flip1995 Jun 20, 2024

flip1995 Jun 19, 2024 •

edited

Loading

tgross35 Jun 19, 2024

tgross35 commented Jun 19, 2024

flip1995 commented Jun 20, 2024

bors commented Jun 20, 2024

tgross35 commented Jun 20, 2024

Resolve Clippy f16 and f128 unimplemented!/FIXMEs #126636

Resolve Clippy f16 and f128 unimplemented!/FIXMEs #126636

Conversation

tgross35 commented Jun 18, 2024 • edited Loading

rustbot commented Jun 18, 2024

tgross35 commented Jun 18, 2024 • edited Loading

tgross35 Jun 18, 2024

Choose a reason for hiding this comment

flip1995 Jun 19, 2024

Choose a reason for hiding this comment

tgross35 Jun 19, 2024

Choose a reason for hiding this comment

flip1995 Jun 20, 2024

Choose a reason for hiding this comment

flip1995 Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

tgross35 Jun 19, 2024

Choose a reason for hiding this comment

tgross35 commented Jun 19, 2024

flip1995 commented Jun 20, 2024

bors commented Jun 20, 2024

tgross35 commented Jun 20, 2024

Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s #126636

Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s #126636

tgross35 commented Jun 18, 2024 •

edited

Loading

tgross35 commented Jun 18, 2024 •

edited

Loading

flip1995 Jun 19, 2024 •

edited

Loading