Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 33 additions & 14 deletions compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let Some(from_trait) = self.tcx.lang_items().from_trait() else {
return UnordSet::new();
};
let obligations = self.fulfillment_cx.borrow().pending_obligations();
let obligations = self
.fulfillment_cx
.borrow()
.pending_obligations_potentially_referencing_float_infer(self);
debug!(?obligations);
let mut vids = UnordSet::new();
for obligation in obligations {
Expand All @@ -209,6 +212,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// Using an intentionally low depth to minimize the chance of future
/// breaking changes in case we adapt the approach later on. This also
/// avoids any hangs for exponentially growing proof trees.
const MAX_DEPTH_FOR_OBLIGATIONS_VISITORS: usize = 5;

struct NestedObligationsForSelfTy<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
self_ty: ty::TyVid,
Expand All @@ -223,10 +231,7 @@ impl<'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'_, 'tcx> {
}

fn config(&self) -> InspectConfig {
// Using an intentionally low depth to minimize the chance of future
// breaking changes in case we adapt the approach later on. This also
// avoids any hangs for exponentially growing proof trees.
InspectConfig { max_depth: 5 }
InspectConfig { max_depth: MAX_DEPTH_FOR_OBLIGATIONS_VISITORS }
}

fn visit_goal(&mut self, inspect_goal: &InspectGoal<'_, 'tcx>) {
Expand Down Expand Up @@ -282,23 +287,37 @@ impl<'tcx> ProofTreeVisitor<'tcx> for FindFromFloatForF32RootVids<'_, 'tcx> {
}

fn config(&self) -> InspectConfig {
// Avoid hang from exponentially growing proof trees (see `cycle-modulo-ambig-aliases.rs`).
// 3 is more than enough for all occurrences in practice (a.k.a. `Into`).
InspectConfig { max_depth: 3 }
InspectConfig { max_depth: MAX_DEPTH_FOR_OBLIGATIONS_VISITORS }
}

fn visit_goal(&mut self, inspect_goal: &InspectGoal<'_, 'tcx>) {
// No need to walk into goal subtrees that certainly hold, since they
// wouldn't then be stalled on an infer var.
if inspect_goal.result() == Ok(Certainty::Yes) {
return;
}

// We don't care about any pending goals which don't actually
// use any float infer var.
if !inspect_goal
.orig_values()
.iter()
.filter_map(|arg| arg.as_type())
.any(|ty| matches!(self.fcx.shallow_resolve(ty).kind(), ty::Infer(ty::FloatVar(_))))
{
debug!(goal = ?inspect_goal.goal(), "goal does not mention float infer var");
return;
}

if let Some(vid) = self
.fcx
.predicate_from_float_for_f32_root_vid(self.from_trait, inspect_goal.goal().predicate)
{
self.vids.insert(vid);
} else if let Some(candidate) = inspect_goal.unique_applicable_candidate() {
let start_len = self.vids.len();
let _ = candidate.goal().infcx().commit_if_ok(|_| {
candidate.visit_nested_no_probe(self);
if self.vids.len() > start_len { Ok(()) } else { Err(()) }
});
}

if let Some(candidate) = inspect_goal.unique_applicable_candidate() {
candidate.visit_nested_no_probe(self);
}
}
}
11 changes: 11 additions & 0 deletions compiler/rustc_infer/src/traits/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ pub trait TraitEngine<'tcx, E: 'tcx>: 'tcx {
self.pending_obligations()
}

/// Pending obligations potentially referencing float inference variables.
///
/// FIXME: use a generic filter for `pending_obligations_potentially_referencing_sub_root`
/// and this after `TraitEngine` doesn't need to be dyn compatible.
fn pending_obligations_potentially_referencing_float_infer(
&self,
_infcx: &InferCtxt<'tcx>,
) -> PredicateObligations<'tcx> {
self.pending_obligations()
}

/// Among all pending obligations, collect those are stalled on a inference variable which has
/// changed since the last call to `try_evaluate_obligations`. Those obligations are marked as
/// successful and returned.
Expand Down
74 changes: 45 additions & 29 deletions compiler/rustc_trait_selection/src/solve/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_infer::traits::query::NoSolution;
use rustc_infer::traits::{
FromSolverError, PredicateObligation, PredicateObligations, TraitEngine,
};
use rustc_middle::ty::{self, TyCtxt, TyVid, TypeVisitableExt, TypingMode};
use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt, TypingMode};
use rustc_next_trait_solver::solve::fast_path::compute_goal_fast_path;
use rustc_next_trait_solver::solve::{
GoalEvaluation, GoalStalledOn, HasChanged, MaybeInfo, SolverDelegateEvalExt as _,
Expand Down Expand Up @@ -79,33 +79,12 @@ impl<'tcx> ObligationStorage<'tcx> {
obligations
}

fn clone_pending_potentially_referencing_sub_root(
&self,
infcx: &InferCtxt<'tcx>,
vid: TyVid,
) -> PredicateObligations<'tcx> {
let mut obligations: PredicateObligations<'tcx> = self
.pending
.iter()
.filter(|(_, stalled_on)| {
let Some(stalled_on) = stalled_on else { return true };
// Don't reuse the sub-unification roots cached on `stalled_on`:
// a later sub-unification merge can have changed which root
// each stalled var belongs to, so the cached info can be stale.
// Walk `stalled_vars` and recompute the current root instead.
//
// Conservative here: if a stalled var no longer resolves to an
// infer var, some unification happened, so the goal is no longer
// stalled. Include it to be re-evaluated downstream.
stalled_on.stalled_vars.iter().filter_map(|arg| arg.as_type()).any(
|ty| match *infcx.shallow_resolve(ty).kind() {
ty::Infer(ty::TyVar(tv)) => infcx.sub_unification_table_root_var(tv) == vid,
_ => true,
},
)
})
.map(|(o, _)| o.clone())
.collect();
fn clone_pending_filtered<F>(&self, f: F) -> PredicateObligations<'tcx>
where
F: FnMut(&&(PredicateObligation<'tcx>, Option<GoalStalledOn<TyCtxt<'tcx>>>)) -> bool,
{
let mut obligations: PredicateObligations<'tcx> =
self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect();
obligations.extend(self.overflowed.iter().cloned());
obligations
}
Expand Down Expand Up @@ -310,7 +289,44 @@ where
if infcx.tcx.disable_trait_solver_fast_paths() {
return self.obligations.clone_pending();
}
self.obligations.clone_pending_potentially_referencing_sub_root(infcx, vid)
self.obligations.clone_pending_filtered(|(_, stalled_on)| {
let Some(stalled_on) = stalled_on else { return true };
// Don't reuse the sub-unification roots cached on `stalled_on`:
// a later sub-unification merge can have changed which root
// each stalled var belongs to, so the cached info can be stale.
// Walk `stalled_vars` and recompute the current root instead.
//
// Conservative here: if a stalled var no longer resolves to an
// infer var, some unification happened, so the goal is no longer
// stalled. Include it to be re-evaluated downstream.
stalled_on.stalled_vars.iter().filter_map(|arg| arg.as_type()).any(|ty| {
match *infcx.shallow_resolve(ty).kind() {
ty::Infer(ty::TyVar(tv)) => infcx.sub_unification_table_root_var(tv) == vid,
_ => true,
}
})
})
}

fn pending_obligations_potentially_referencing_float_infer(
&self,
infcx: &InferCtxt<'tcx>,
) -> PredicateObligations<'tcx> {
// `-Zdisable-fast-paths`: same gate as the other new-solver fast paths.
if infcx.tcx.disable_trait_solver_fast_paths() {
return self.obligations.clone_pending();
}

self.obligations.clone_pending_filtered(|(_, stalled_on)| {
let Some(stalled_on) = stalled_on else { return true };
// If the stalled vars don't have float infers, the nested goals won't
// have them either. We only create float infers for user written literals.
stalled_on
.stalled_vars
.iter()
.filter_map(|arg| arg.as_type())
.any(|ty| matches!(infcx.shallow_resolve(ty).kind(), ty::Infer(ty::FloatVar(_))))
})
}

fn drain_stalled_obligations_for_coroutines(
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/traits/next-solver/float-fallback-hack-depth.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//@ compile-flags: -Znext-solver
//@ check-pass

// Regression test for github.com/rust-lang/trait-system-refactor-initiative#280
// We force unresolved float infer vars to fallback to `f32` if there're stalled `f32: From<?float>`
// obligations.
// Previously the recursion limit is 3 which is not enough, causing some bevy crates to fail.

trait Trait {}
impl<T: Into<f32>> Trait for T {}

struct W<T>(T);
impl<T: Trait> Trait for W<T> {}

fn impls_trait<T: Trait>(_: T) {}

fn main() {
impls_trait(W(1.0))
//~^ WARN: falling back to `f32` as the trait bound `f32: From<f64>` is not satisfied [float_literal_f32_fallback]
//~| WARN: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}
12 changes: 12 additions & 0 deletions tests/ui/traits/next-solver/float-fallback-hack-depth.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
warning: falling back to `f32` as the trait bound `f32: From<f64>` is not satisfied
--> $DIR/float-fallback-hack-depth.rs:18:19
|
LL | impls_trait(W(1.0))
| ^^^ help: explicitly specify the type as `f32`: `1.0_f32`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #154024 <https://github.com/rust-lang/rust/issues/154024>
= note: `#[warn(float_literal_f32_fallback)]` (part of `#[warn(future_incompatible)]`) on by default

warning: 1 warning emitted

Loading