Skip to content

Use ExtractIf in fulfillment loop #144699

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
8 changes: 8 additions & 0 deletions compiler/rustc_next_trait_solver/src/delegate.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use std::ops::Deref;

use rustc_type_ir::inherent::*;
use rustc_type_ir::solve::{Certainty, Goal, NoSolution};
use rustc_type_ir::{self as ty, InferCtxtLike, Interner, TypeFoldable};

use crate::solve::GoalStalledOn;

pub trait SolverDelegate: Deref<Target = Self::Infcx> + Sized {
type Infcx: InferCtxtLike<Interner = Self::Interner>;
type Interner: Interner;
Expand All @@ -23,6 +26,11 @@ pub trait SolverDelegate: Deref<Target = Self::Infcx> + Sized {
span: <Self::Interner as Interner>::Span,
) -> Option<Certainty>;

fn is_still_stalled(&self, stalled_on: &GoalStalledOn<Self::Interner>) -> bool {
!stalled_on.stalled_vars.iter().any(|value| self.is_changed_arg(*value))
&& !self.opaque_types_storage_num_entries().needs_reevaluation(stalled_on.num_opaques)
}

fn fresh_var_for_kind_with_span(
&self,
arg: <Self::Interner as Interner>::GenericArg,
Expand Down
37 changes: 27 additions & 10 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,11 +432,7 @@ where
// args have changed. Otherwise, we don't need to re-run the goal because it'll remain
// stalled, since it'll canonicalize the same way and evaluation is pure.
if let Some(stalled_on) = stalled_on
&& !stalled_on.stalled_vars.iter().any(|value| self.delegate.is_changed_arg(*value))
&& !self
.delegate
.opaque_types_storage_num_entries()
.needs_reevaluation(stalled_on.num_opaques)
&& self.delegate.is_still_stalled(&stalled_on)
{
return Ok((
NestedNormalizationGoals::empty(),
Expand Down Expand Up @@ -639,15 +635,24 @@ where
///
/// Goals for the next step get directly added to the nested goals of the `EvalCtxt`.
fn evaluate_added_goals_step(&mut self) -> Result<Option<Certainty>, NoSolution> {
if self.nested_goals.is_empty() {
return Ok(Some(Certainty::Yes));
}

let cx = self.cx();
// If this loop did not result in any progress, what's our final certainty.
let mut unchanged_certainty = Some(Certainty::Yes);
for (source, goal, stalled_on) in mem::take(&mut self.nested_goals) {

let mut nested_goals = mem::take(&mut self.nested_goals);
let mut pending_goals = vec![];
for (source, goal, stalled_on) in nested_goals.extract_if(.., |(_, _, stalled_on)| {
stalled_on.as_ref().is_none_or(|s| !self.delegate.is_still_stalled(s))
}) {
if let Some(certainty) = self.delegate.compute_goal_fast_path(goal, self.origin_span) {
match certainty {
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.nested_goals.push((source, goal, None));
pending_goals.push((source, goal, None));
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
}
}
Expand Down Expand Up @@ -684,7 +689,7 @@ where
)?;
// Add the nested goals from normalization to our own nested goals.
trace!(?nested_goals);
self.nested_goals.extend(nested_goals.into_iter().map(|(s, g)| (s, g, None)));
pending_goals.extend(nested_goals.into_iter().map(|(s, g)| (s, g, None)));

// Finally, equate the goal's RHS with the unconstrained var.
//
Expand Down Expand Up @@ -728,7 +733,7 @@ where
match certainty {
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.nested_goals.push((source, with_resolved_vars, stalled_on));
pending_goals.push((source, with_resolved_vars, stalled_on));
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
}
}
Expand All @@ -742,13 +747,25 @@ where
match certainty {
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.nested_goals.push((source, goal, stalled_on));
pending_goals.push((source, goal, stalled_on));
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
}
}
}
}

// Nested goals still need to be accounted for in the `unchanged_certainty`.
for (_, _, stalled_on) in &nested_goals {
if let Some(GoalStalledOn { stalled_cause, .. }) = stalled_on {
unchanged_certainty =
unchanged_certainty.map(|c| c.and(Certainty::Maybe(*stalled_cause)));
}
}

debug_assert!(self.nested_goals.is_empty());
nested_goals.extend(pending_goals);
self.nested_goals = nested_goals;

Ok(unchanged_certainty)
}

Expand Down
101 changes: 47 additions & 54 deletions compiler/rustc_trait_selection/src/solve/fulfill.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::marker::PhantomData;
use std::mem;
use std::ops::ControlFlow;

use rustc_data_structures::thinvec::ExtractIf;
Expand Down Expand Up @@ -75,6 +74,13 @@ impl<'tcx> ObligationStorage<'tcx> {
self.pending.push((obligation, stalled_on));
}

fn register_overflowed(
&mut self,
overflowed: impl IntoIterator<Item = PredicateObligation<'tcx>>,
) {
self.overflowed.extend(overflowed);
}

fn has_pending_obligations(&self) -> bool {
!self.pending.is_empty() || !self.overflowed.is_empty()
}
Expand All @@ -88,35 +94,14 @@ impl<'tcx> ObligationStorage<'tcx> {

fn drain_pending(
&mut self,
cond: impl Fn(&PredicateObligation<'tcx>) -> bool,
) -> PendingObligations<'tcx> {
let (unstalled, pending) =
mem::take(&mut self.pending).into_iter().partition(|(o, _)| cond(o));
self.pending = pending;
unstalled
cond: impl Fn(&PredicateObligation<'tcx>, Option<&GoalStalledOn<TyCtxt<'tcx>>>) -> bool,
) -> impl Iterator<Item = (PredicateObligation<'tcx>, Option<GoalStalledOn<TyCtxt<'tcx>>>)>
{
ExtractIf::new(&mut self.pending, move |(o, stalled)| cond(o, stalled.as_ref()))
}

fn on_fulfillment_overflow(&mut self, infcx: &InferCtxt<'tcx>) {
infcx.probe(|_| {
// IMPORTANT: we must not use solve any inference variables in the obligations
// as this is all happening inside of a probe. We use a probe to make sure
// we get all obligations involved in the overflow. We pretty much check: if
// we were to do another step of `select_where_possible`, which goals would
// change.
// FIXME: <https://github.com/Gankra/thin-vec/pull/66> is merged, this can be removed.
self.overflowed.extend(
ExtractIf::new(&mut self.pending, |(o, stalled_on)| {
let goal = o.as_goal();
let result = <&SolverDelegate<'tcx>>::from(infcx).evaluate_root_goal(
goal,
o.cause.span,
stalled_on.take(),
);
matches!(result, Ok(GoalEvaluation { has_changed: HasChanged::Yes, .. }))
})
.map(|(o, _)| o),
);
})
fn num_pending(&self) -> usize {
self.pending.len()
}
}

Expand All @@ -133,21 +118,6 @@ impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> {
_errors: PhantomData,
}
}

fn inspect_evaluated_obligation(
&self,
infcx: &InferCtxt<'tcx>,
obligation: &PredicateObligation<'tcx>,
result: &Result<GoalEvaluation<TyCtxt<'tcx>>, NoSolution>,
) {
if let Some(inspector) = infcx.obligation_inspector.get() {
let result = match result {
Ok(GoalEvaluation { certainty, .. }) => Ok(*certainty),
Err(NoSolution) => Err(NoSolution),
};
(inspector)(infcx, &obligation, result);
}
}
}

impl<'tcx, E> TraitEngine<'tcx, E> for FulfillmentCtxt<'tcx, E>
Expand Down Expand Up @@ -180,19 +150,27 @@ where
}

fn select_where_possible(&mut self, infcx: &InferCtxt<'tcx>) -> Vec<E> {
if self.obligations.num_pending() == 0 {
return vec![];
}

assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots());
let delegate = <&SolverDelegate<'tcx>>::from(infcx);
let mut errors = Vec::new();
loop {
let mut any_changed = false;
for (mut obligation, stalled_on) in self.obligations.drain_pending(|_| true) {
let mut overflowed = vec![];
let mut pending = vec![];

for (mut obligation, stalled_on) in self.obligations.drain_pending(|_, stalled_on| {
stalled_on.is_none_or(|s| !delegate.is_still_stalled(s))
}) {
if !infcx.tcx.recursion_limit().value_within_limit(obligation.recursion_depth) {
self.obligations.on_fulfillment_overflow(infcx);
// Only return true errors that we have accumulated while processing.
return errors;
overflowed.push(obligation);
continue;
}

let goal = obligation.as_goal();
let delegate = <&SolverDelegate<'tcx>>::from(infcx);
if let Some(certainty) =
delegate.compute_goal_fast_path(goal, obligation.cause.span)
{
Expand All @@ -204,15 +182,21 @@ where
//
// Only goals proven via the trait solver should be region dependent.
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.obligations.register(obligation, None);
}
Certainty::Maybe(_) => pending.push((obligation, None)),
}
continue;
}

let result = delegate.evaluate_root_goal(goal, obligation.cause.span, stalled_on);
self.inspect_evaluated_obligation(infcx, &obligation, &result);

if let Some(inspector) = infcx.obligation_inspector.get() {
let result = match result {
Ok(GoalEvaluation { certainty, .. }) => Ok(certainty),
Err(NoSolution) => Err(NoSolution),
};
(inspector)(infcx, &obligation, result);
}

let GoalEvaluation { goal, certainty, has_changed, stalled_on } = match result {
Ok(result) => result,
Err(NoSolution) => {
Expand Down Expand Up @@ -256,10 +240,19 @@ where
infcx.push_hir_typeck_potentially_region_dependent_goal(obligation);
}
}
Certainty::Maybe(_) => self.obligations.register(obligation, stalled_on),
Certainty::Maybe(_) => pending.push((obligation, stalled_on)),
}
}

if !overflowed.is_empty() {
self.obligations.register_overflowed(overflowed);
return errors;
}

for (obligation, stalled_on) in pending {
self.obligations.register(obligation, stalled_on);
}

if !any_changed {
break;
}
Expand Down Expand Up @@ -295,7 +288,7 @@ where
}

self.obligations
.drain_pending(|obl| {
.drain_pending(|obl, _| {
infcx.probe(|_| {
infcx
.visit_proof_tree(
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/impl-trait/recursive-in-exhaustiveness.next.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ LL | build(x)
| ^^^^^^^^ cannot normalize `build<_>::{opaque#0}`

error[E0271]: type mismatch resolving `build2<(_,)>::{opaque#0} normalizes-to _`
--> $DIR/recursive-in-exhaustiveness.rs:30:6
--> $DIR/recursive-in-exhaustiveness.rs:30:5
|
LL | (build2(x),)
| ^^^^^^^^^ types differ
| ^^^^^^^^^^^^ types differ

error[E0271]: type mismatch resolving `build2<(_,)>::{opaque#0} normalizes-to _`
--> $DIR/recursive-in-exhaustiveness.rs:30:5
--> $DIR/recursive-in-exhaustiveness.rs:30:6
|
LL | (build2(x),)
| ^^^^^^^^^^^^ types differ
| ^^^^^^^^^ types differ

error[E0277]: the size for values of type `(impl Sized,)` cannot be known at compilation time
--> $DIR/recursive-in-exhaustiveness.rs:30:5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0282]: type annotations needed
--> $DIR/two_tait_defining_each_other2.rs:12:11
|
LL | fn muh(x: A) -> B {
| ^ cannot infer type
| ^ cannot infer type for type alias `A`

error: aborting due to 1 previous error

Expand Down
1 change: 1 addition & 0 deletions tests/ui/traits/next-solver/alias-bound-unsound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ fn main() {
//~^ ERROR overflow evaluating the requirement `String <: <() as Foo>::Item`
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item well-formed`
//~| ERROR overflow evaluating the requirement `&<() as Foo>::Item well-formed`
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item: Sized`
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item == _`
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item == _`
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item == _`
Expand Down
10 changes: 9 additions & 1 deletion tests/ui/traits/next-solver/alias-bound-unsound.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ LL | drop(<() as Foo>::copy_me(&x));
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error[E0275]: overflow evaluating the requirement `<() as Foo>::Item: Sized`
--> $DIR/alias-bound-unsound.rs:24:10
|
LL | drop(<() as Foo>::copy_me(&x));
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the return type of a function must have a statically known size

error[E0275]: overflow evaluating the requirement `&<() as Foo>::Item well-formed`
--> $DIR/alias-bound-unsound.rs:24:31
|
Expand All @@ -58,6 +66,6 @@ error[E0275]: overflow evaluating the requirement `<() as Foo>::Item == _`
LL | drop(<() as Foo>::copy_me(&x));
| ^^

error: aborting due to 8 previous errors
error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0275`.
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
error[E0283]: type annotations needed
error[E0284]: type annotations needed: cannot normalize `<_ as Iterator>::Item`
--> $DIR/runaway-impl-candidate-selection.rs:13:22
|
LL | println!("{:?}", iter::<_>());
| ^^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `iter`
|
= note: cannot satisfy `_: Iterator`
note: required by a bound in `iter`
--> $DIR/runaway-impl-candidate-selection.rs:8:12
|
LL | fn iter<T: Iterator>() -> <T as Iterator>::Item {
| ^^^^^^^^ required by this bound in `iter`
| ^^^^^^^^^^^ cannot normalize `<_ as Iterator>::Item`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0283`.
For more information about this error, try `rustc --explain E0284`.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ LL | impl<T: TwoW> Trait for W<T> {}
| ---------------------------- first implementation here
LL | impl<T: TwoW> Trait for T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>>>`
|
= note: overflow evaluating the requirement `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>>>: TwoW`
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "20"]` attribute to your crate (`coherence_fulfill_overflow`)

error: aborting due to 1 previous error

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/traits/next-solver/coroutine.fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ LL | | },
| |_________^ the trait `Coroutine<A>` is not implemented for `{coroutine@$DIR/coroutine.rs:20:9: 20:11}`
|
note: required by a bound in `needs_coroutine`
--> $DIR/coroutine.rs:14:28
--> $DIR/coroutine.rs:14:41
|
LL | fn needs_coroutine(_: impl Coroutine<A, Yield = B, Return = C>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `needs_coroutine`
| ^^^^^^^^^ required by this bound in `needs_coroutine`

error: aborting due to 1 previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ error[E0277]: the trait bound `A<X>: Trait<_, _, _>` is not satisfied
LL | impls_trait::<A<X>, _, _, _>();
| ^^^^ the trait `Trait<_, _, _>` is not implemented for `A<X>`
|
= help: the trait `Trait<U, V, D>` is implemented for `A<T>`
= help: the trait `Trait<_, _, _>` is not implemented for `A<X>`
but trait `Trait<u32, u8, u8>` is implemented for it
note: required for `A<X>` to implement `Trait<_, _, _>`
--> $DIR/incompleteness-unstable-result.rs:34:18
|
Expand Down
Loading
Loading