Skip to content
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

Return nested obligations from canonical response var unification #109493

Merged
merged 2 commits into from
Mar 24, 2023
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
22 changes: 8 additions & 14 deletions compiler/rustc_trait_selection/src/solve/canonical/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,20 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
original_values: Vec<ty::GenericArg<'tcx>>,
response: CanonicalResponse<'tcx>,
) -> Result<Certainty, NoSolution> {
) -> Result<(Certainty, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution> {
let substitution = self.compute_query_response_substitution(&original_values, &response);

let Response { var_values, external_constraints, certainty } =
response.substitute(self.tcx(), &substitution);

self.unify_query_var_values(param_env, &original_values, var_values)?;
let nested_goals = self.unify_query_var_values(param_env, &original_values, var_values)?;

// FIXME: implement external constraints.
let ExternalConstraintsData { region_constraints, opaque_types: _ } =
external_constraints.deref();
self.register_region_constraints(region_constraints);

Ok(certainty)
Ok((certainty, nested_goals))
}

/// This returns the substitutions to instantiate the bound variables of
Expand Down Expand Up @@ -205,21 +205,15 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
original_values: &[ty::GenericArg<'tcx>],
var_values: CanonicalVarValues<'tcx>,
) -> Result<(), NoSolution> {
) -> Result<Vec<Goal<'tcx, ty::Predicate<'tcx>>>, NoSolution> {
assert_eq!(original_values.len(), var_values.len());

let mut nested_goals = vec![];
for (&orig, response) in iter::zip(original_values, var_values.var_values) {
// This can fail due to the occurs check, see
// `tests/ui/typeck/lazy-norm/equating-projection-cyclically.rs` for an example
// where that can happen.
//
// FIXME: To deal with #105787 I also expect us to emit nested obligations here at
// some point. We can figure out how to deal with this once we actually have
// an ICE.
let nested_goals = self.eq_and_get_goals(param_env, orig, response)?;
assert!(nested_goals.is_empty(), "{nested_goals:?}");
nested_goals.extend(self.eq_and_get_goals(param_env, orig, response)?);
}

Ok(())
Ok(nested_goals)
}

fn register_region_constraints(&mut self, region_constraints: &QueryRegionConstraints<'tcx>) {
Expand Down
17 changes: 9 additions & 8 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,16 @@ pub trait InferCtxtEvalExt<'tcx> {
fn evaluate_root_goal(
&self,
goal: Goal<'tcx, ty::Predicate<'tcx>>,
) -> Result<(bool, Certainty), NoSolution>;
) -> Result<(bool, Certainty, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution>;
}

impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> {
#[instrument(level = "debug", skip(self))]
fn evaluate_root_goal(
&self,
goal: Goal<'tcx, ty::Predicate<'tcx>>,
) -> Result<(bool, Certainty), NoSolution> {
) -> Result<(bool, Certainty, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution> {
let mode = if self.intercrate { SolverMode::Coherence } else { SolverMode::Normal };

let mut search_graph = search_graph::SearchGraph::new(self.tcx, mode);

let mut ecx = EvalCtxt {
Expand Down Expand Up @@ -152,13 +151,13 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
&mut self,
is_normalizes_to_hack: IsNormalizesToHack,
goal: Goal<'tcx, ty::Predicate<'tcx>>,
) -> Result<(bool, Certainty), NoSolution> {
) -> Result<(bool, Certainty, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution> {
let (orig_values, canonical_goal) = self.canonicalize_goal(goal);
let canonical_response =
EvalCtxt::evaluate_canonical_goal(self.tcx(), self.search_graph, canonical_goal)?;

let has_changed = !canonical_response.value.var_values.is_identity();
let certainty = self.instantiate_and_apply_query_response(
let (certainty, nested_goals) = self.instantiate_and_apply_query_response(
goal.param_env,
orig_values,
canonical_response,
Expand Down Expand Up @@ -186,7 +185,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
assert_eq!(certainty, canonical_response.value.certainty);
}

Ok((has_changed, certainty))
Ok((has_changed, certainty, nested_goals))
}

fn compute_goal(&mut self, goal: Goal<'tcx, ty::Predicate<'tcx>>) -> QueryResult<'tcx> {
Expand Down Expand Up @@ -263,13 +262,14 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
let mut has_changed = Err(Certainty::Yes);

if let Some(goal) = goals.normalizes_to_hack_goal.take() {
let (_, certainty) = match this.evaluate_goal(
let (_, certainty, nested_goals) = match this.evaluate_goal(
IsNormalizesToHack::Yes,
goal.with(this.tcx(), ty::Binder::dummy(goal.predicate)),
) {
Ok(r) => r,
Err(NoSolution) => return Some(Err(NoSolution)),
};
new_goals.goals.extend(nested_goals);

if goal.predicate.projection_ty
!= this.resolve_vars_if_possible(goal.predicate.projection_ty)
Expand Down Expand Up @@ -308,11 +308,12 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
}

for nested_goal in goals.goals.drain(..) {
let (changed, certainty) =
let (changed, certainty, nested_goals) =
match this.evaluate_goal(IsNormalizesToHack::No, nested_goal) {
Ok(result) => result,
Err(NoSolution) => return Some(Err(NoSolution)),
};
new_goals.goals.extend(nested_goals);

if changed {
has_changed = Ok(());
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_trait_selection/src/solve/fulfill.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::mem;

use rustc_infer::infer::InferCtxt;
use rustc_infer::traits::Obligation;
use rustc_infer::traits::{
query::NoSolution, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes,
PredicateObligation, SelectionError, TraitEngine,
Expand Down Expand Up @@ -61,7 +62,7 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentCtxt<'tcx> {
let mut has_changed = false;
for obligation in mem::take(&mut self.obligations) {
let goal = obligation.clone().into();
let (changed, certainty) = match infcx.evaluate_root_goal(goal) {
let (changed, certainty, nested_goals) = match infcx.evaluate_root_goal(goal) {
Ok(result) => result,
Err(NoSolution) => {
errors.push(FulfillmentError {
Expand Down Expand Up @@ -125,7 +126,16 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentCtxt<'tcx> {
continue;
}
};

// Push any nested goals that we get from unifying our canonical response
// with our obligation onto the fulfillment context.
self.obligations.extend(nested_goals.into_iter().map(|goal| {
Obligation::new(
infcx.tcx,
obligation.cause.clone(),
goal.param_env,
goal.predicate,
)
}));
has_changed |= changed;
match certainty {
Certainty::Yes => {}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use rustc_middle::traits::solve::{Certainty, Goal, MaybeCause};
use rustc_infer::traits::{TraitEngine, TraitEngineExt};
use rustc_middle::ty;

use crate::infer::canonical::OriginalQueryValues;
use crate::infer::InferCtxt;
use crate::solve::InferCtxtEvalExt;
use crate::traits::{EvaluationResult, OverflowError, PredicateObligation, SelectionContext};

pub trait InferCtxtExt<'tcx> {
Expand Down Expand Up @@ -81,27 +80,20 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {

if self.tcx.trait_solver_next() {
self.probe(|snapshot| {
if let Ok((_, certainty)) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we already talked about that before, but is there a reason we can't use the FulfillmentContext and select_where_possible?

Copy link
Member Author

@compiler-errors compiler-errors Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't bubble up overflow correctly when using a fulfillment context, but we're actually already using a fulfillment context for the evaluate queries in SelectionCtxt.... so seems alright for now, I can migrate it to use fulfillment. I'll leave a FIXME.

self.evaluate_root_goal(Goal::new(self.tcx, param_env, obligation.predicate))
{
match certainty {
Certainty::Yes => {
if self.opaque_types_added_in_snapshot(snapshot) {
Ok(EvaluationResult::EvaluatedToOkModuloOpaqueTypes)
} else if self.region_constraints_added_in_snapshot(snapshot).is_some()
{
Ok(EvaluationResult::EvaluatedToOkModuloRegions)
} else {
Ok(EvaluationResult::EvaluatedToOk)
}
}
Certainty::Maybe(MaybeCause::Ambiguity) => {
Ok(EvaluationResult::EvaluatedToAmbig)
}
Certainty::Maybe(MaybeCause::Overflow) => Err(OverflowError::Canonical),
}
} else {
let mut fulfill_cx = crate::solve::FulfillmentCtxt::new();
fulfill_cx.register_predicate_obligation(self, obligation.clone());
// True errors
// FIXME(-Ztrait-solver=next): Overflows are reported as ambig here, is that OK?
if !fulfill_cx.select_where_possible(self).is_empty() {
Ok(EvaluationResult::EvaluatedToErr)
} else if !fulfill_cx.select_all_or_error(self).is_empty() {
Ok(EvaluationResult::EvaluatedToAmbig)
} else if self.opaque_types_added_in_snapshot(snapshot) {
Ok(EvaluationResult::EvaluatedToOkModuloOpaqueTypes)
} else if self.region_constraints_added_in_snapshot(snapshot).is_some() {
Ok(EvaluationResult::EvaluatedToOkModuloRegions)
} else {
Ok(EvaluationResult::EvaluatedToOk)
}
})
} else {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let mut fulfill_cx = crate::solve::FulfillmentCtxt::new();
fulfill_cx.register_predicate_obligations(self.infcx, predicates);
// True errors
// FIXME(-Ztrait-solver=next): Overflows are reported as ambig here, is that OK?
if !fulfill_cx.select_where_possible(self.infcx).is_empty() {
return Ok(EvaluatedToErr);
}
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/traits/new-solver/alias-eq-in-canonical-response.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// check-pass
// compile-flags: -Ztrait-solver=next

trait Foo {
type Gat<'a>
where
Self: 'a;
fn bar(&self) -> Self::Gat<'_>;
}

enum Option<T> {
Some(T),
None,
}

impl<T> Option<T> {
fn as_ref(&self) -> Option<&T> {
match self {
Option::Some(t) => Option::Some(t),
Option::None => Option::None,
}
}

fn map<U>(self, f: impl FnOnce(T) -> U) -> Option<U> {
match self {
Option::Some(t) => Option::Some(f(t)),
Option::None => Option::None,
}
}
}

impl<T: Foo + 'static> Foo for Option<T> {
type Gat<'a> = Option<<T as Foo>::Gat<'a>> where Self: 'a;

fn bar(&self) -> Self::Gat<'_> {
self.as_ref().map(Foo::bar)
}
}

fn main() {}