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

[beetrees]

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]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[compiler-errors]

@bors try

[bors]

⌛ Trying commit f4cb5a7 with merge 3ec2a77...

[bors]

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

[compiler-errors]

@craterbot check

[craterbot]

👌 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]

🚧 Experiment pr-139087 is now running

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

[craterbot]

🚨 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]

@craterbot retry-report

[craterbot]

🛠️ 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]

🎉 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

[traviscross]

I guess, for me to feel comfortable about this, I would want the FCW to land first, and to see 1) that the impact of that is not too large and 2) that it causes a significant reduction in the use of this fallback, indicating a strong likelihood that we will eventually be able to remove this hack.

Agree with @jackh726 here, and would add that ideally it'd work well enough that we don't need to add the hack at all.

[tgross35]

I think Boxy may have hinted at this, but is implementation complexity across both solvers the reason we can't do a more general solution at this time? At a surface level it seems "easy" to solve both the float and int examples by trying T (i.e. no conversion) if Into<T> is ambiguous. But I admittedly know very little about the inner workings here.

[bors]

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

[tgross35]

Talked to @compiler-errors about the more general fallback at all hands. My understanding of the problem is that by the time the solver knows there is an unfulfilled obligation, it is already done solving and has no way to backtrack. So (aiui) it might be possible to infer the type of the literal at the location of the Into bound, but if it’s assigned to a variable then other things relying on the type of the variable would still act as if the type is unknown.

@Amanieu suggested there might be a way to keep track of the two possible solutions.

[traviscross]

In the spirit of brainstorming, I wonder whether pattern types could ever have a role to play here. E.g., in

fn f<T: Into<u32>>(_: T) {}
fn main() {
    f(1);
}

maybe we could say that the literal falls back to i32 is 1..=1, and then we have an impl From<i32 is 0..> for u32. (This wouldn't allow literals bigger than i32::MAX to work, but maybe that's OK.)

Of course, pattern types are hard, and are I'm sure especially challenging to combine with floats.

cc @oli-obk

[rustbot]

changes to inspect_obligations.rs

cc @compiler-errors, @lcnr

[beetrees]

I guess, for me to feel comfortable about this, I would want the FCW to land first, and to see 1) that the impact of that is not too large and 2) that it causes a significant reduction in the use of this fallback, indicating a strong likelihood that we will eventually be able to remove this hack.

Adding a FCW without the trait impl would require a completely different implementation as the FCW as implemented relies on the float var reaching the fallback stage in order to warn about it. As the added trait impl (From<f16> for f32) will only be available on nightly until f16 is stabilised, this PR could be reverted at any point until f16 is stabilised without breaking backwards compatibility guarantees - is that sufficient? In regards to (1): I referenced a Crater run in #139087 (comment) from a year ago which had root regressions in 42 crates and 52 GitHub repos, but I'm happy to put up a PR with just the From<f16> for f32 impl to run Crater on if more recent results are desired.

maybe we could say that the literal falls back to i32 is 1..=1, and then we have an impl From<i32 is 0..> for u32. (This wouldn't allow literals bigger than i32::MAX to work, but maybe that's OK.)

Of course, pattern types are hard, and are I'm sure especially challenging to combine with floats.

I don't think this would work for float literals due to double rounding.

[beetrees]

There seems to be a bit of a split of opinion between "add a FCW and eventually remove the f32 fallback" and "don't add a FCW and expand the fallback so that it works for all literals constrained by From<{float}> for fN or From<{integer}> for iN/uN". I'm guessing this needs to be nominated lang team and/or types team discussion to decide which direction to head in?

[beetrees]

I've opened a PR to run Crater to measure the impact of the trait impl and FCW at #142723.

[bors]

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