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

Resolve f16 and f128 unimplemented!/FIXMEs #12950

Closed
wants to merge 4 commits into from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Jun 18, 2024

This removes the ICE codepaths for f16 and f128. 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.

changelog: make Clippy work correctly with f16 and f128

(do not merge, contains toolchain updates for testing)

@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2024

r? @blyxyas

rustbot has assigned @blyxyas.
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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 18, 2024
@tgross35 tgross35 force-pushed the f16-f128-fixme branch 2 times, most recently from a72f40d to af397ce Compare June 18, 2024 02:39
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should I do here? Functions aren't yet available to convert and use f128 for everything instead.

@tgross35 tgross35 marked this pull request as draft June 18, 2024 02:46
@tgross35
Copy link
Contributor Author

Marking a draft since I still need to update tests

@tgross35 tgross35 force-pushed the f16-f128-fixme branch 6 times, most recently from 08a85ed to 91a40c1 Compare June 18, 2024 03:49
@tgross35
Copy link
Contributor Author

I updated the tests to add f16 and f128 everywhere it seemed relevant (i.e. everywhere where both f32 and f64 had tests). A lot of these need to be left as FIXMEs though since they rely on functions that are not yet implemented, or const eval casting which isn't done yet.

CI testing will continue to fail until the nightly it uses gets bumped to something from the last couple days or newer (after const arithmetic)

@flip1995
Copy link
Member

flip1995 commented Jun 18, 2024

See my comment here: #12952 (comment)

TL;DR: I would just include those changes in the Rust PR

…tations

This removes the ICE codepaths for `f16` and `f128`. `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 tgross35 changed the title Replace f16 and f128 unimplemented!/FIXMEs with real implementations Resolve f16 and f128 unimplemented!/FIXMEs Jun 18, 2024
@tgross35
Copy link
Contributor Author

tgross35 commented Jun 18, 2024

Thank you for taking a look!

Do you know what might be happening at https://github.com/rust-lang/rust-clippy/actions/runs/9569119207/job/26380925098?pr=12950#step:5:1055? It looks like the fix is just deleting the entire file for some reason. fixed that

@flip1995
Copy link
Member

Closing in favor of rust-lang/rust#126636

Thanks for moving it there!

@flip1995 flip1995 closed this Jun 19, 2024
@tgross35 tgross35 deleted the f16-f128-fixme branch June 19, 2024 17:01
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 20, 2024
…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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 20, 2024
…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
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 20, 2024
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
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 28, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants