Skip to content

Commit e8e4541

Browse files
committed
Auto merge of rust-lang#139087 - beetrees:impl-from-f16-for-f32, r=jackh726
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 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 rust-lang#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](rust-lang#139087 (comment)). 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: 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 cc rust-lang#154024 rust-lang#154005
2 parents 9620eae + a759013 commit e8e4541

31 files changed

+601
-32
lines changed

compiler/rustc_hir/src/lang_items.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,9 @@ language_item_table! {
443443
FieldBase, sym::field_base, field_base, Target::AssocTy, GenericRequirement::Exact(0);
444444
FieldType, sym::field_type, field_type, Target::AssocTy, GenericRequirement::Exact(0);
445445
FieldOffset, sym::field_offset, field_offset, Target::AssocConst, GenericRequirement::Exact(0);
446+
447+
// Used to fallback `{float}` to `f32` when `f32: From<{float}>`
448+
From, sym::From, from_trait, Target::Trait, GenericRequirement::Exact(1);
446449
}
447450

448451
/// The requirement imposed on the generics of a lang item

compiler/rustc_hir_typeck/src/demand.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
346346
match infer {
347347
ty::TyVar(_) => self.next_ty_var(DUMMY_SP),
348348
ty::IntVar(_) => self.next_int_var(),
349-
ty::FloatVar(_) => self.next_float_var(),
349+
ty::FloatVar(_) => self.next_float_var(DUMMY_SP, None),
350350
ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_) => {
351351
bug!("unexpected fresh ty outside of the trait solver")
352352
}

compiler/rustc_hir_typeck/src/errors.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,3 +1384,15 @@ pub(crate) struct ProjectOnNonPinProjectType {
13841384
)]
13851385
pub sugg_span: Option<Span>,
13861386
}
1387+
1388+
#[derive(Diagnostic)]
1389+
#[diag("falling back to `f32` as the trait bound `f32: From<f64>` is not satisfied")]
1390+
pub(crate) struct FloatLiteralF32Fallback {
1391+
pub literal: String,
1392+
#[suggestion(
1393+
"explicitly specify the type as `f32`",
1394+
code = "{literal}_f32",
1395+
applicability = "machine-applicable"
1396+
)]
1397+
pub span: Option<Span>,
1398+
}

compiler/rustc_hir_typeck/src/expr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
349349

350350
let tcx = self.tcx;
351351
match expr.kind {
352-
ExprKind::Lit(ref lit) => self.check_expr_lit(lit, expected),
352+
ExprKind::Lit(ref lit) => self.check_expr_lit(lit, expr.hir_id, expected),
353353
ExprKind::Binary(op, lhs, rhs) => self.check_expr_binop(expr, op, lhs, rhs, expected),
354354
ExprKind::Assign(lhs, rhs, span) => {
355355
self.check_expr_assign(expr, expected, lhs, rhs, span)

compiler/rustc_hir_typeck/src/fallback.rs

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ use rustc_data_structures::fx::FxHashSet;
55
use rustc_data_structures::graph;
66
use rustc_data_structures::graph::vec_graph::VecGraph;
77
use rustc_data_structures::unord::{UnordMap, UnordSet};
8-
use rustc_hir as hir;
9-
use rustc_hir::HirId;
108
use rustc_hir::attrs::DivergingFallbackBehavior;
119
use rustc_hir::def::{DefKind, Res};
1210
use rustc_hir::def_id::DefId;
1311
use rustc_hir::intravisit::{InferKind, Visitor};
14-
use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable};
12+
use rustc_hir::{self as hir, CRATE_HIR_ID, HirId};
13+
use rustc_lint::builtin::FLOAT_LITERAL_F32_FALLBACK;
14+
use rustc_middle::ty::{self, FloatVid, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable};
1515
use rustc_session::lint;
1616
use rustc_span::def_id::LocalDefId;
1717
use rustc_span::{DUMMY_SP, Span};
@@ -55,15 +55,20 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
5555

5656
let (diverging_fallback, diverging_fallback_ty) =
5757
self.calculate_diverging_fallback(&unresolved_variables);
58+
let fallback_to_f32 = self.calculate_fallback_to_f32(&unresolved_variables);
5859

5960
// We do fallback in two passes, to try to generate
6061
// better error messages.
6162
// The first time, we do *not* replace opaque types.
6263
let mut fallback_occurred = false;
6364
for ty in unresolved_variables {
6465
debug!("unsolved_variable = {:?}", ty);
65-
fallback_occurred |=
66-
self.fallback_if_possible(ty, &diverging_fallback, diverging_fallback_ty);
66+
fallback_occurred |= self.fallback_if_possible(
67+
ty,
68+
&diverging_fallback,
69+
diverging_fallback_ty,
70+
&fallback_to_f32,
71+
);
6772
}
6873

6974
fallback_occurred
@@ -73,7 +78,8 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
7378
///
7479
/// - Unconstrained ints are replaced with `i32`.
7580
///
76-
/// - Unconstrained floats are replaced with `f64`.
81+
/// - Unconstrained floats are replaced with `f64`, except when there is a trait predicate
82+
/// `f32: From<{float}>`, in which case `f32` is used as the fallback instead.
7783
///
7884
/// - Non-numerics may get replaced with `()` or `!`, depending on how they
7985
/// were categorized by [`Self::calculate_diverging_fallback`], crate's
@@ -89,6 +95,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
8995
ty: Ty<'tcx>,
9096
diverging_fallback: &UnordSet<Ty<'tcx>>,
9197
diverging_fallback_ty: Ty<'tcx>,
98+
fallback_to_f32: &UnordSet<FloatVid>,
9299
) -> bool {
93100
// Careful: we do NOT shallow-resolve `ty`. We know that `ty`
94101
// is an unsolved variable, and we determine its fallback
@@ -111,6 +118,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
111118
let fallback = match ty.kind() {
112119
_ if let Some(e) = self.tainted_by_errors() => Ty::new_error(self.tcx, e),
113120
ty::Infer(ty::IntVar(_)) => self.tcx.types.i32,
121+
ty::Infer(ty::FloatVar(vid)) if fallback_to_f32.contains(vid) => self.tcx.types.f32,
114122
ty::Infer(ty::FloatVar(_)) => self.tcx.types.f64,
115123
_ if diverging_fallback.contains(&ty) => {
116124
self.diverging_fallback_has_occurred.set(true);
@@ -125,6 +133,52 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
125133
true
126134
}
127135

136+
/// Existing code relies on `f32: From<T>` (usually written as `T: Into<f32>`) resolving `T` to
137+
/// `f32` when the type of `T` is inferred from an unsuffixed float literal. Using the default
138+
/// fallback of `f64`, this would break when adding `impl From<f16> for f32`, as there are now
139+
/// two float type which could be `T`, meaning that the fallback of `f64` would be used and
140+
/// compilation error would occur as `f32` does not implement `From<f64>`. To avoid breaking
141+
/// existing code, we instead fallback `T` to `f32` when there is a trait predicate
142+
/// `f32: From<T>`. This means code like the following will continue to compile:
143+
///
144+
/// ```rust
145+
/// fn foo<T: Into<f32>>(_: T) {}
146+
///
147+
/// foo(1.0);
148+
/// ```
149+
fn calculate_fallback_to_f32(&self, unresolved_variables: &[Ty<'tcx>]) -> UnordSet<FloatVid> {
150+
let roots: UnordSet<ty::FloatVid> = self.from_float_for_f32_root_vids();
151+
if roots.is_empty() {
152+
// Most functions have no `f32: From<{float}>` predicates, so short-circuit and return
153+
// an empty set when this is the case.
154+
return UnordSet::new();
155+
}
156+
// Calculate all the unresolved variables that need to fallback to `f32` here. This ensures
157+
// we don't need to find root variables in `fallback_if_possible`: see the comment at the
158+
// top of that function for details.
159+
let fallback_to_f32 = unresolved_variables
160+
.iter()
161+
.flat_map(|ty| ty.float_vid())
162+
.filter(|vid| roots.contains(&self.root_float_var(*vid)))
163+
.inspect(|vid| {
164+
let origin = self.float_var_origin(*vid);
165+
// Show the entire literal in the suggestion to make it clearer.
166+
let literal = self.tcx.sess.source_map().span_to_snippet(origin.span).ok();
167+
self.tcx.emit_node_span_lint(
168+
FLOAT_LITERAL_F32_FALLBACK,
169+
origin.lint_id.unwrap_or(CRATE_HIR_ID),
170+
origin.span,
171+
errors::FloatLiteralF32Fallback {
172+
span: literal.as_ref().map(|_| origin.span),
173+
literal: literal.unwrap_or_default(),
174+
},
175+
);
176+
})
177+
.collect();
178+
debug!("calculate_fallback_to_f32: fallback_to_f32={:?}", fallback_to_f32);
179+
fallback_to_f32
180+
}
181+
128182
fn calculate_diverging_fallback(
129183
&self,
130184
unresolved_variables: &[Ty<'tcx>],
@@ -362,6 +416,11 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
362416
Some(self.root_var(self.shallow_resolve(ty).ty_vid()?))
363417
}
364418

419+
/// If `ty` is an unresolved float type variable, returns its root vid.
420+
pub(crate) fn root_float_vid(&self, ty: Ty<'tcx>) -> Option<ty::FloatVid> {
421+
Some(self.root_float_var(self.shallow_resolve(ty).float_vid()?))
422+
}
423+
365424
/// Given a set of diverging vids and coercions, walk the HIR to gather a
366425
/// set of suggestions which can be applied to preserve fallback to unit.
367426
fn try_to_suggest_annotations(

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
718718
pub(in super::super) fn check_expr_lit(
719719
&self,
720720
lit: &hir::Lit,
721+
lint_id: HirId,
721722
expected: Expectation<'tcx>,
722723
) -> Ty<'tcx> {
723724
let tcx = self.tcx;
@@ -765,7 +766,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
765766
ty::Float(_) => Some(ty),
766767
_ => None,
767768
});
768-
opt_ty.unwrap_or_else(|| self.next_float_var())
769+
opt_ty.unwrap_or_else(|| self.next_float_var(lit.span, Some(lint_id)))
769770
}
770771
ast::LitKind::Bool(_) => tcx.types.bool,
771772
ast::LitKind::CStr(_, _) => Ty::new_imm_ref(

compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! A utility module to inspect currently ambiguous obligations in the current context.
22
3+
use rustc_data_structures::unord::UnordSet;
4+
use rustc_hir::def_id::DefId;
35
use rustc_infer::traits::{self, ObligationCause, PredicateObligations};
46
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
57
use rustc_span::Span;
@@ -96,6 +98,69 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
9698
});
9799
obligations_for_self_ty
98100
}
101+
102+
/// Only needed for the `From<{float}>` for `f32` type fallback.
103+
#[instrument(skip(self), level = "debug")]
104+
pub(crate) fn from_float_for_f32_root_vids(&self) -> UnordSet<ty::FloatVid> {
105+
if self.next_trait_solver() {
106+
self.from_float_for_f32_root_vids_next()
107+
} else {
108+
let Some(from_trait) = self.tcx.lang_items().from_trait() else {
109+
return UnordSet::new();
110+
};
111+
self.fulfillment_cx
112+
.borrow_mut()
113+
.pending_obligations()
114+
.into_iter()
115+
.filter_map(|obligation| {
116+
self.predicate_from_float_for_f32_root_vid(from_trait, obligation.predicate)
117+
})
118+
.collect()
119+
}
120+
}
121+
122+
fn predicate_from_float_for_f32_root_vid(
123+
&self,
124+
from_trait: DefId,
125+
predicate: ty::Predicate<'tcx>,
126+
) -> Option<ty::FloatVid> {
127+
// The predicates we are looking for look like
128+
// `TraitPredicate(<f32 as std::convert::From<{float}>>, polarity:Positive)`.
129+
// They will have no bound variables.
130+
match predicate.kind().no_bound_vars() {
131+
Some(ty::PredicateKind::Clause(ty::ClauseKind::Trait(ty::TraitPredicate {
132+
polarity: ty::PredicatePolarity::Positive,
133+
trait_ref,
134+
}))) if trait_ref.def_id == from_trait
135+
&& self.shallow_resolve(trait_ref.self_ty()).kind()
136+
== &ty::Float(ty::FloatTy::F32) =>
137+
{
138+
self.root_float_vid(trait_ref.args.type_at(1))
139+
}
140+
_ => None,
141+
}
142+
}
143+
144+
fn from_float_for_f32_root_vids_next(&self) -> UnordSet<ty::FloatVid> {
145+
let Some(from_trait) = self.tcx.lang_items().from_trait() else {
146+
return UnordSet::new();
147+
};
148+
let obligations = self.fulfillment_cx.borrow().pending_obligations();
149+
debug!(?obligations);
150+
let mut vids = UnordSet::new();
151+
for obligation in obligations {
152+
let mut visitor = FindFromFloatForF32RootVids {
153+
fcx: self,
154+
from_trait,
155+
vids: &mut vids,
156+
span: obligation.cause.span,
157+
};
158+
159+
let goal = obligation.as_goal();
160+
self.visit_proof_tree(goal, &mut visitor);
161+
}
162+
vids
163+
}
99164
}
100165

101166
struct NestedObligationsForSelfTy<'a, 'tcx> {
@@ -105,7 +170,7 @@ struct NestedObligationsForSelfTy<'a, 'tcx> {
105170
obligations_for_self_ty: &'a mut PredicateObligations<'tcx>,
106171
}
107172

108-
impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> {
173+
impl<'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'_, 'tcx> {
109174
fn span(&self) -> Span {
110175
self.root_cause.span
111176
}
@@ -144,3 +209,37 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> {
144209
}
145210
}
146211
}
212+
213+
struct FindFromFloatForF32RootVids<'a, 'tcx> {
214+
fcx: &'a FnCtxt<'a, 'tcx>,
215+
from_trait: DefId,
216+
vids: &'a mut UnordSet<ty::FloatVid>,
217+
span: Span,
218+
}
219+
220+
impl<'tcx> ProofTreeVisitor<'tcx> for FindFromFloatForF32RootVids<'_, 'tcx> {
221+
fn span(&self) -> Span {
222+
self.span
223+
}
224+
225+
fn config(&self) -> InspectConfig {
226+
// Avoid hang from exponentially growing proof trees (see `cycle-modulo-ambig-aliases.rs`).
227+
// 3 is more than enough for all occurrences in practice (a.k.a. `Into`).
228+
InspectConfig { max_depth: 3 }
229+
}
230+
231+
fn visit_goal(&mut self, inspect_goal: &InspectGoal<'_, 'tcx>) {
232+
if let Some(vid) = self
233+
.fcx
234+
.predicate_from_float_for_f32_root_vid(self.from_trait, inspect_goal.goal().predicate)
235+
{
236+
self.vids.insert(vid);
237+
} else if let Some(candidate) = inspect_goal.unique_applicable_candidate() {
238+
let start_len = self.vids.len();
239+
let _ = candidate.goal().infcx().commit_if_ok(|_| {
240+
candidate.visit_nested_no_probe(self);
241+
if self.vids.len() > start_len { Ok(()) } else { Err(()) }
242+
});
243+
}
244+
}
245+
}

compiler/rustc_hir_typeck/src/pat.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
917917
fn check_pat_expr_unadjusted(&self, lt: &'tcx hir::PatExpr<'tcx>) -> Ty<'tcx> {
918918
let ty = match &lt.kind {
919919
rustc_hir::PatExprKind::Lit { lit, negated } => {
920-
let ty = self.check_expr_lit(lit, Expectation::NoExpectation);
920+
let ty = self.check_expr_lit(lit, lt.hir_id, Expectation::NoExpectation);
921921
if *negated {
922922
self.register_bound(
923923
ty,

compiler/rustc_infer/src/infer/canonical/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,10 @@ impl<'tcx> InferCtxt<'tcx> {
107107

108108
CanonicalVarKind::Int => self.next_int_var().into(),
109109

110-
CanonicalVarKind::Float => self.next_float_var().into(),
110+
CanonicalVarKind::Float => {
111+
// There is no HirId available to pass as a lint_id.
112+
self.next_float_var(span, None).into()
113+
}
111114

112115
CanonicalVarKind::PlaceholderTy(ty::PlaceholderType { universe, bound, .. }) => {
113116
let universe_mapped = universe_map(universe);

0 commit comments

Comments
 (0)