-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
f32::from(<untyped float>)
inference with f16
and f128
#123831
Comments
@rustbot label -needs-triage |
There's no way we can provide an impl conditional on edition, since those are always global and can't really be conditional based on the way that the trait solver works. The way that this worked for We still may be able to hack around this, but I would strongly urge first doing the analysis of the fallout from breaking inference here, since any hacks that we need to introduce need to exist for approximately forever, which is a long time. |
Too bad, but that makes enough sense. I guess it would be interesting to see crater results, though the fact that this was discovered within 24 hours of merge makes me think there could be quite a lot of codebases relying on this somewhere... @rustbot label -A-edition-2024 |
Crater run at #122470 (comment) |
Some languages give literals their own special type for making from-literal conversions more tractable to steer in the type system. |
Some kind of improvement to type inference such that it can handle this case would seem ideal. Perhaps even a lang feature to allow user code to guide the inference process. From a library author's position this change is awkward because it is not necessarily possible to fix it without making a semver-breaking update. But it also seems non-great if these API additions are blocked indefinitely. |
Can the link to the Taffy regression be updated to https://github.com/DioxusLabs/taffy/blob/f1e4edab96ee13f7d1fbc2536ac31633b0e620d6/src/style_helpers.rs#L58, before it was fixed in DioxusLabs/taffy@9f27870? |
I'm also getting the same warnings for this line, this line, this line, this line, and these lines. Most of them are related to casting a tuple I hope this helps :) |
Crater finished https://crater-reports.s3.amazonaws.com/pr-122470/index.html |
Hm -- looks like a lot of these are due to As I stated above, impls need to apply globally, so it's gonna be a bit harder to fix this (if possible at all, without hacking out something that I'd be afraid of having to support in perpetuity). |
Yeah, I would rather not have this impl than have something specific and hacky. How feasible would it be for untyped literals to select their type based on the bound? As in, if there is a requirement for fn foo(a: impl Into<u64>) {}
fn main() {
foo(100);
} |
Re-add `From<f16> for f64` This impl was originally added in rust-lang#122470 before being removed in rust-lang#123830 due to rust-lang#123831. However, the issue only affects `f32` (which currently only has one `From<{float}>` impl, `From<f32>`) as `f64` already has two `From<{float}>` impls (`From<f32>` and `From<f64>`) and is also the float literal fallback type anyway. Therefore it is safe to re-add `From<f16> for f64`. This PR also updates the FIXME link to point to the open issue rust-lang#123831 rather than the closed issue rust-lang#123824. Tracking issue: rust-lang#116909 `@rustbot` label +F-f16_and_f128 +T-libs-api
Re-add `From<f16> for f64` This impl was originally added in rust-lang#122470 before being removed in rust-lang#123830 due to rust-lang#123831. However, the issue only affects `f32` (which currently only has one `From<{float}>` impl, `From<f32>`) as `f64` already has two `From<{float}>` impls (`From<f32>` and `From<f64>`) and is also the float literal fallback type anyway. Therefore it is safe to re-add `From<f16> for f64`. This PR also updates the FIXME link to point to the open issue rust-lang#123831 rather than the closed issue rust-lang#123824. Tracking issue: rust-lang#116909 `@rustbot` label +F-f16_and_f128 +T-libs-api
Adding a
From<f16> for f32
impl breaks some inference that currently works. From #123824:The new float types are a long way from being stable, but it may be good to decide how to handle things if we would like this implementation one day. Per @fmease at #123824 (comment), there are three options:
From<f16> for f32
I'll mark this with the edition label so it can at least come into consideration, but this is the lowest priority thing on any list.
Known failure points from adding this directly:
f32
#123824 (comment))taffy
: https://github.com/DioxusLabs/taffy/blob/f1e4edab96ee13f7d1fbc2536ac31633b0e620d6/src/style_helpers.rs#L58 (from Type inference failure forf32
#123824 (comment))contour-rs
: https://github.com/mthh/contour-rs/blob/08c1d83a1461aa19543ed402d23908eb55234fd1/src/lib.rs#L488-L491 (from Fix overflow when using large raster sizes mthh/contour-rs#13 (comment))f32::from(<untyped float>)
inference withf16
andf128
#123831 (comment)the trait bound f32: From<f64> is not satisfied
error on nightly emilk/egui#4352@rustbot label +T-lang +T-libs +A-edition-2024 +F-f16_and_f128
The text was updated successfully, but these errors were encountered: