Fallback `{float}` to `f32` when `f32: From<{float}>` and add `impl From<f16> for f32` #139087

beetrees · 2025-03-28T21:15:30Z

Currently, the following code compiles:

fn foo<T: Into<f32>>(_: T) {}

fn main() {
    foo(1.0);
}

This is because the only From<{float}> impl for f32 is currently From<f32>. However, once impl From<f16> for f32 is added this is no longer the case. This would cause the float literal to fallback to f64, subsequently causing a type error as f32 does not implement From<f64>. While this kind of change to type inference isn't technically a breaking change according to Rust's breaking change policy, the previous attempt to add impl From<f16> for f32 was removed #123830 due to the large number of crates affected (by my count, there were root regressions in 42 crates and 52 GitHub repos, not including duplicates). This PR solves this problem by using f32 as the fallback type for {float} when there is a trait predicate of f32: From<{float}>. This allows adding impl From<f16> for f32 without affecting the code that currently compiles (such as the example above; this PR shouldn't affect what is possible on stable).

This PR also allows adding a future-incompatibility warning for the fallback to f32 (currently implemented in the third commit) if the lang team wants one (allowing the f32 fallback to be removed in the future); alternatively this could be expanded in the future into something more general like @tgross35 suggested in #123831 (comment). I think it would be also possible to disallow the f32 fallback in a future edition.

As expected, a crater check showed no non-spurious regressions.

For reference, I've based the implementation loosely on the existing calculate_diverging_fallback. This first commit adds the f32 fallback, the second adds impl From<f16> for f32, and the third adds a FCW lint for the f32 fallback. I think this falls under the types team, so
r? types

Fixes: #123831
Tracking issue: #116909

@rustbot label +T-lang +T-types +T-libs-api +F-f16_and_f128

To decide on whether a future-incompatibility warning is desired or otherwise (see above):
@rustbot label +I-lang-nominated

rustbot · 2025-03-28T21:15:36Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler-errors · 2025-03-28T23:33:02Z

@bors try

bors · 2025-03-28T23:34:13Z

⌛ Trying commit f4cb5a7 with merge 3ec2a77...

Fallback `{float}` to `f32` when `f32: From<{float}>` and add `impl From<f16> for f32` Currently, the following code compiles: ```rust fn foo::<T: Into<f32>>(_: T) {} fn main() { foo(1.0); } ``` This is because the only `From<{float}>` impl for `f32` is currently `From<f32>`. However, once `impl From<f16> for f32` is added this is no longer the case. This would cause the float literal to fallback to `f64`, subsequently causing a type error as `f32` does not implement `From<f64>`. While this kind of change to type inference isn't technically a breaking change according to Rust's breaking change policy, the previous attempt to add `impl From<f16> for f32` was removed rust-lang#123830 due to the large number of crates affected (by my count, there were root regressions in 42 crates and 52 GitHub repos, not including duplicates). This PR solves this problem by using `f32` as the fallback type for `{float}` when there is a trait predicate of `f32: From<{float}>`. This allows adding `impl From<f16> for f32` without affecting the code that currently compiles (such as the example above; this PR shouldn't affect what is possible on stable). This PR also allows adding a future-incompatibility warning if the lang team wants one; alternatively this could be expanded in the future into something more general like `@tgross35` suggested in rust-lang#123831 (comment). I think it would be also possible to disallow the `f32` fallback in a future edition. This will need a crater check. For reference, I've based the implementation loosely on the existing `calculate_diverging_fallback`. This first commit adds the `f32` fallback and the second adds `impl From<f16> for f32`. I think this falls under the types team, so r? types Fixes: rust-lang#123831 Tracking issue: rust-lang#116909 `@rustbot` label +T-lang +T-types +T-libs-api +F-f16_and_f128 To decide on whether a future-incompatibility warning is desired or otherwise (see above): `@rustbot` label +I-lang-nominated

bors · 2025-03-29T01:41:45Z

☀️ Try build successful - checks-actions
Build commit: 3ec2a77 (3ec2a775459d36c7d90654e47c56ff7bb0d542ab)

compiler-errors · 2025-03-29T16:08:48Z

@craterbot check

craterbot · 2025-03-29T16:09:03Z

👌 Experiment pr-139087 created and queued.
🤖 Automatically detected try build 3ec2a77
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-03-29T16:10:13Z

🚧 Experiment pr-139087 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-03-30T11:57:43Z

🚨 Report generation of pr-139087 failed: timed out waiting for connection
🛠️ If the error is fixed use the retry-report command.

🆘 Can someone from the infra team check in on this? @rust-lang/infra
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

compiler-errors · 2025-03-30T16:13:37Z

@craterbot retry-report

craterbot · 2025-03-30T16:13:39Z

🛠️ Generation of the report for pr-139087 queued again.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-03-30T18:41:20Z

🎉 Experiment pr-139087 is completed!
📊 9 regressed and 2 fixed (605729 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

beetrees · 2025-03-30T20:30:14Z

As expected, all regressions are spurious:

1 caused by the failure of build script detection of u128/i128 support (AFAICT for some reason num-traits didn't detect support but num-integer did).
5 CMake IO errors.
1 error downloading packages.
2 docker errors.

lcnr · 2025-03-31T09:49:27Z

compiler/rustc_hir_typeck/src/fallback.rs

+            .iter()
+            .flat_map(|ty| ty.float_vid())
+            .filter(|vid| roots.contains(&self.root_float_var(*vid)))
+            .collect();


Why do the mapping from root to all unified infer vars here instead of looking at the root in fallback_if_possible? Please add a comment

I've added a comment. I put that there because that's what calculate_diverging_fallback does and there's a comment at the top of fallback_if_possible stating that "we determine its fallback based solely on how it was created, not what other type variables it may have been unified with since then", saying it makes it easier to detect bugs.

lcnr · 2025-03-31T09:50:13Z

Somewhat hacky, but that feels required unfortunately 😅 this PR looks good to me

Unlike an additional general fallback mechanism, this is not a breaking change as f32: From<{float}> currently only compiles when inferring to f32. So there is no way to end up with the case of "some other fallback requires the a newly fallback'd var to be something different from its fallback".

I would like us to future compat warn here and to potentially remove this fallback in the future, or future editions. I think our current fallback rules are kinda iffy and could imagine us wanting to change our approach in more fundamental ways at some point in the future.

beetrees · 2025-03-31T22:23:36Z

I've added the future-incompatibility warning in the third commit. I've left the issue number as FIXME for now but will open an actual issue for it before this PR is merged.

bors · 2025-04-04T05:46:44Z

☔ The latest upstream changes (presumably #120706) made this pull request unmergeable. Please resolve the merge conflicts.

beetrees · 2025-04-04T07:34:20Z

Rebased

rustbot assigned lcnr Mar 28, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 28, 2025

beetrees force-pushed the impl-from-f16-for-f32 branch from ef59666 to bb79c28 Compare March 28, 2025 21:34

This comment has been minimized.

Sign in to view

beetrees force-pushed the impl-from-f16-for-f32 branch from bb79c28 to afe5b7f Compare March 28, 2025 22:35

This comment has been minimized.

Sign in to view

beetrees force-pushed the impl-from-f16-for-f32 branch from afe5b7f to f4cb5a7 Compare March 28, 2025 23:32

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2025

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Mar 30, 2025

lcnr added needs-fcp This change is insta-stable, so needs a completed FCP to proceed. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 31, 2025

lcnr reviewed Mar 31, 2025

View reviewed changes

beetrees force-pushed the impl-from-f16-for-f32 branch from f4cb5a7 to 7cf6e0c Compare March 31, 2025 22:18

This comment has been minimized.

Sign in to view

beetrees force-pushed the impl-from-f16-for-f32 branch from 7cf6e0c to 7cbd8ef Compare March 31, 2025 22:58

beetrees added 3 commits April 4, 2025 08:25

Fallback {float} to f32 when f32: From<{float}>

398469b

Add impl From<f16> for f32

8e9a78c

Add FCW for unsuffixed float literal f32 fallback

b089102

beetrees force-pushed the impl-from-f16-for-f32 branch from 7cbd8ef to b089102 Compare April 4, 2025 07:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fallback `{float}` to `f32` when `f32: From<{float}>` and add `impl From<f16> for f32` #139087

Fallback `{float}` to `f32` when `f32: From<{float}>` and add `impl From<f16> for f32` #139087

beetrees commented Mar 28, 2025 •

edited

Loading

rustbot commented Mar 28, 2025

This comment has been minimized.

This comment has been minimized.

compiler-errors commented Mar 28, 2025

bors commented Mar 28, 2025

bors commented Mar 29, 2025

compiler-errors commented Mar 29, 2025

craterbot commented Mar 29, 2025

craterbot commented Mar 29, 2025

craterbot commented Mar 30, 2025

compiler-errors commented Mar 30, 2025

craterbot commented Mar 30, 2025

craterbot commented Mar 30, 2025

beetrees commented Mar 30, 2025 •

edited

Loading

lcnr Mar 31, 2025

beetrees Mar 31, 2025

lcnr commented Mar 31, 2025 •

edited

Loading

beetrees commented Mar 31, 2025 •

edited

Loading

This comment has been minimized.

bors commented Apr 4, 2025

beetrees commented Apr 4, 2025

Fallback {float} to f32 when f32: From<{float}> and add impl From<f16> for f32 #139087

Are you sure you want to change the base?

Fallback {float} to f32 when f32: From<{float}> and add impl From<f16> for f32 #139087

Conversation

beetrees commented Mar 28, 2025 • edited Loading

rustbot commented Mar 28, 2025

This comment has been minimized.

This comment has been minimized.

compiler-errors commented Mar 28, 2025

bors commented Mar 28, 2025

bors commented Mar 29, 2025

compiler-errors commented Mar 29, 2025

craterbot commented Mar 29, 2025

craterbot commented Mar 29, 2025

craterbot commented Mar 30, 2025

compiler-errors commented Mar 30, 2025

craterbot commented Mar 30, 2025

craterbot commented Mar 30, 2025

beetrees commented Mar 30, 2025 • edited Loading

lcnr Mar 31, 2025

Choose a reason for hiding this comment

beetrees Mar 31, 2025

Choose a reason for hiding this comment

lcnr commented Mar 31, 2025 • edited Loading

beetrees commented Mar 31, 2025 • edited Loading

This comment has been minimized.

bors commented Apr 4, 2025

beetrees commented Apr 4, 2025

Fallback `{float}` to `f32` when `f32: From<{float}>` and add `impl From<f16> for f32` #139087

Fallback `{float}` to `f32` when `f32: From<{float}>` and add `impl From<f16> for f32` #139087

beetrees commented Mar 28, 2025 •

edited

Loading

beetrees commented Mar 30, 2025 •

edited

Loading

lcnr commented Mar 31, 2025 •

edited

Loading

beetrees commented Mar 31, 2025 •

edited

Loading