Skip to content

implement intercrate_ambiguity_causes in the new solver #115996

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

Merged
merged 6 commits into from
Sep 21, 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
66 changes: 58 additions & 8 deletions compiler/rustc_middle/src/traits/solve/inspect.rs
Original file line number Diff line number Diff line change
@@ -1,30 +1,70 @@
//! Data structure used to inspect trait solver behavior.
//!
//! During trait solving we optionally build "proof trees", the root of
//! which is a [GoalEvaluation] with [GoalEvaluationKind::Root]. These
//! trees are used to improve the debug experience and are also used by
//! the compiler itself to provide necessary context for error messages.
//!
//! Because each nested goal in the solver gets [canonicalized] separately
//! and we discard inference progress via "probes", we cannot mechanically
//! use proof trees without somehow "lifting up" data local to the current
//! `InferCtxt`. Any data used mechanically is therefore canonicalized and
//! stored as [CanonicalState]. As printing canonicalized data worsens the
//! debugging dumps, we do not simply canonicalize everything.
//!
//! This means proof trees contain inference variables and placeholders
//! local to a different `InferCtxt` which must not be used with the
//! current one.
//!
//! [canonicalized]: https://rustc-dev-guide.rust-lang.org/solve/canonicalization.html

use super::{
CandidateSource, CanonicalInput, Certainty, Goal, IsNormalizesToHack, NoSolution, QueryInput,
QueryResult,
CandidateSource, Canonical, CanonicalInput, Certainty, Goal, IsNormalizesToHack, NoSolution,
QueryInput, QueryResult,
};
use crate::ty;
use crate::{infer::canonical::CanonicalVarValues, ty};
use format::ProofTreeFormatter;
use std::fmt::{Debug, Write};

mod format;

/// Some `data` together with information about how they relate to the input
/// of the canonical query.
///
/// This is only ever used as [CanonicalState]. Any type information in proof
/// trees used mechanically has to be canonicalized as we otherwise leak
/// inference variables from a nested `InferCtxt`.
#[derive(Debug, Clone, Copy, Eq, PartialEq, TypeFoldable, TypeVisitable)]
pub struct State<'tcx, T> {
pub var_values: CanonicalVarValues<'tcx>,
pub data: T,
}

pub type CanonicalState<'tcx, T> = Canonical<'tcx, State<'tcx, T>>;

#[derive(Debug, Eq, PartialEq)]
pub enum CacheHit {
Provisional,
Global,
}

/// When evaluating the root goals we also store the
/// original values for the `CanonicalVarValues` of the
/// canonicalized goal. We use this to map any [CanonicalState]
/// from the local `InferCtxt` of the solver query to
/// the `InferCtxt` of the caller.
#[derive(Eq, PartialEq)]
pub enum GoalEvaluationKind {
Root,
pub enum GoalEvaluationKind<'tcx> {
Root { orig_values: Vec<ty::GenericArg<'tcx>> },
Nested { is_normalizes_to_hack: IsNormalizesToHack },
}

#[derive(Eq, PartialEq)]
pub struct GoalEvaluation<'tcx> {
pub uncanonicalized_goal: Goal<'tcx, ty::Predicate<'tcx>>,
pub kind: GoalEvaluationKind,
pub kind: GoalEvaluationKind<'tcx>,
pub evaluation: CanonicalGoalEvaluation<'tcx>,
/// The nested goals from instantiating the query response.
pub returned_goals: Vec<Goal<'tcx, ty::Predicate<'tcx>>>,
}

Expand Down Expand Up @@ -66,6 +106,7 @@ pub struct GoalEvaluationStep<'tcx> {
/// of a goal.
#[derive(Eq, PartialEq)]
pub struct Probe<'tcx> {
/// What happened inside of this probe in chronological order.
pub steps: Vec<ProbeStep<'tcx>>,
pub kind: ProbeKind<'tcx>,
}
Expand All @@ -78,12 +119,21 @@ impl Debug for Probe<'_> {

#[derive(Eq, PartialEq)]
pub enum ProbeStep<'tcx> {
AddGoal(Goal<'tcx, ty::Predicate<'tcx>>),
/// We added a goal to the `EvalCtxt` which will get proven
/// the next time `EvalCtxt::try_evaluate_added_goals` is called.
AddGoal(CanonicalState<'tcx, Goal<'tcx, ty::Predicate<'tcx>>>),
/// The inside of a `EvalCtxt::try_evaluate_added_goals` call.
EvaluateGoals(AddedGoalsEvaluation<'tcx>),
/// A call to `probe` while proving the current goal. This is
/// used whenever there are multiple candidates to prove the
/// current goalby .
NestedProbe(Probe<'tcx>),
}

#[derive(Debug, PartialEq, Eq)]
/// What kind of probe we're in. In case the probe represents a candidate, or
/// the final result of the current goal - via [ProbeKind::Root] - we also
/// store the [QueryResult].
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum ProbeKind<'tcx> {
/// The root inference context while proving a goal.
Root { result: QueryResult<'tcx> },
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/traits/solve/inspect/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl<'a, 'b> ProofTreeFormatter<'a, 'b> {

pub(super) fn format_goal_evaluation(&mut self, eval: &GoalEvaluation<'_>) -> std::fmt::Result {
let goal_text = match eval.kind {
GoalEvaluationKind::Root => "ROOT GOAL",
GoalEvaluationKind::Root { orig_values: _ } => "ROOT GOAL",
GoalEvaluationKind::Nested { is_normalizes_to_hack } => match is_normalizes_to_hack {
IsNormalizesToHack::No => "GOAL",
IsNormalizesToHack::Yes => "NORMALIZES-TO HACK GOAL",
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
goal: Goal<'tcx, ty::Predicate<'tcx>>,
) -> Result<(bool, Certainty, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution> {
let (orig_values, canonical_goal) = self.canonicalize_goal(goal);
let mut goal_evaluation = self.inspect.new_goal_evaluation(goal, goal_evaluation_kind);
let mut goal_evaluation =
self.inspect.new_goal_evaluation(goal, &orig_values, goal_evaluation_kind);
let encountered_overflow = self.search_graph.encountered_overflow();
let canonical_response = EvalCtxt::evaluate_canonical_goal(
self.tcx(),
Expand Down Expand Up @@ -568,7 +569,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
GoalEvaluationKind::Nested { is_normalizes_to_hack: IsNormalizesToHack::Yes },
unconstrained_goal,
)?;
self.add_goals(instantiate_goals);
self.nested_goals.goals.extend(instantiate_goals);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I stopped using add_goals here because the added goals aren't really nested goals and it ended up breaking the proof tree builder

Copy link
Member

Choose a reason for hiding this comment

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

Can you be more specific how it breaks things? Like I understand how we probably don't want to include these goals, and I especially understand not wanting to add ambiguous goal repeatedly over and over below, but does this affect tests detrimentally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the issue was that we add goals while the state is DebugBuilder::AddedGoalsEvaluation.

For us to sensibly handle these nested goals we'd have to either add AddGoal as an action to AddedGoalsEvaluation or somehow take the added goals during the evaluation and "move them back" into the WipProbe once the added goals evaluation is done. Both of these feel somewhat ugly and slightly annoying to implement


// Finally, equate the goal's RHS with the unconstrained var.
// We put the nested goals from this into goals instead of
Expand Down Expand Up @@ -605,15 +606,15 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
GoalEvaluationKind::Nested { is_normalizes_to_hack: IsNormalizesToHack::No },
goal,
)?;
self.add_goals(instantiate_goals);
self.nested_goals.goals.extend(instantiate_goals);
if has_changed {
unchanged_certainty = None;
}

match certainty {
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.add_goal(goal);
self.nested_goals.goals.push(goal);
unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
}
}
Expand Down
97 changes: 81 additions & 16 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,21 @@
//! [c]: https://rustc-dev-guide.rust-lang.org/solve/canonicalization.html
use super::{CanonicalInput, Certainty, EvalCtxt, Goal};
use crate::solve::canonicalize::{CanonicalizeMode, Canonicalizer};
use crate::solve::{response_no_constraints_raw, CanonicalResponse, QueryResult, Response};
use crate::solve::{
inspect, response_no_constraints_raw, CanonicalResponse, QueryResult, Response,
};
use rustc_data_structures::fx::FxHashSet;
use rustc_index::IndexVec;
use rustc_infer::infer::canonical::query_response::make_query_region_constraints;
use rustc_infer::infer::canonical::CanonicalVarValues;
use rustc_infer::infer::canonical::{CanonicalExt, QueryRegionConstraints};
use rustc_infer::infer::InferCtxt;
use rustc_infer::infer::{DefineOpaqueTypes, InferCtxt, InferOk};
use rustc_middle::infer::canonical::Canonical;
use rustc_middle::traits::query::NoSolution;
use rustc_middle::traits::solve::{
ExternalConstraintsData, MaybeCause, PredefinedOpaquesData, QueryInput,
};
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::{
self, BoundVar, GenericArgKind, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable,
TypeVisitableExt,
Expand All @@ -29,6 +33,22 @@ use rustc_span::DUMMY_SP;
use std::iter;
use std::ops::Deref;

trait ResponseT<'tcx> {
Copy link
Member

Choose a reason for hiding this comment

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

This name is kinda bad, but I can change it in a follow-up.

fn var_values(&self) -> CanonicalVarValues<'tcx>;
}

impl<'tcx> ResponseT<'tcx> for Response<'tcx> {
fn var_values(&self) -> CanonicalVarValues<'tcx> {
self.var_values
}
}

impl<'tcx, T> ResponseT<'tcx> for inspect::State<'tcx, T> {
fn var_values(&self) -> CanonicalVarValues<'tcx> {
self.var_values
}
}

impl<'tcx> EvalCtxt<'_, 'tcx> {
/// Canonicalizes the goal remembering the original values
/// for each bound variable.
Expand Down Expand Up @@ -188,12 +208,14 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
original_values: Vec<ty::GenericArg<'tcx>>,
response: CanonicalResponse<'tcx>,
) -> Result<(Certainty, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution> {
let substitution = self.compute_query_response_substitution(&original_values, &response);
let substitution =
Self::compute_query_response_substitution(self.infcx, &original_values, &response);

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

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

let ExternalConstraintsData { region_constraints, opaque_types } =
external_constraints.deref();
Expand All @@ -206,21 +228,21 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
/// This returns the substitutions to instantiate the bound variables of
/// the canonical response. This depends on the `original_values` for the
/// bound variables.
fn compute_query_response_substitution(
&self,
fn compute_query_response_substitution<T: ResponseT<'tcx>>(
Copy link
Member

@compiler-errors compiler-errors Sep 20, 2023

Choose a reason for hiding this comment

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

I don't know why this is a method on EvalCtxt anymore in this case. Should probably just be free and live in some public submodule, or even just in solve. You don't have to change this though.

infcx: &InferCtxt<'tcx>,
original_values: &[ty::GenericArg<'tcx>],
response: &CanonicalResponse<'tcx>,
response: &Canonical<'tcx, T>,
) -> CanonicalVarValues<'tcx> {
// FIXME: Longterm canonical queries should deal with all placeholders
// created inside of the query directly instead of returning them to the
// caller.
let prev_universe = self.infcx.universe();
let prev_universe = infcx.universe();
let universes_created_in_query = response.max_universe.index();
for _ in 0..universes_created_in_query {
self.infcx.create_next_universe();
infcx.create_next_universe();
}

let var_values = response.value.var_values;
let var_values = response.value.var_values();
assert_eq!(original_values.len(), var_values.len());

// If the query did not make progress with constraining inference variables,
Expand Down Expand Up @@ -254,13 +276,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
}
}

let var_values = self.tcx().mk_args_from_iter(response.variables.iter().enumerate().map(
let var_values = infcx.tcx.mk_args_from_iter(response.variables.iter().enumerate().map(
|(index, info)| {
if info.universe() != ty::UniverseIndex::ROOT {
// A variable from inside a binder of the query. While ideally these shouldn't
// exist at all (see the FIXME at the start of this method), we have to deal with
// them for now.
self.infcx.instantiate_canonical_var(DUMMY_SP, info, |idx| {
infcx.instantiate_canonical_var(DUMMY_SP, info, |idx| {
ty::UniverseIndex::from(prev_universe.index() + idx.index())
})
} else if info.is_existential() {
Expand All @@ -274,7 +296,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
if let Some(v) = opt_values[BoundVar::from_usize(index)] {
v
} else {
self.infcx.instantiate_canonical_var(DUMMY_SP, info, |_| prev_universe)
infcx.instantiate_canonical_var(DUMMY_SP, info, |_| prev_universe)
}
} else {
// For placeholders which were already part of the input, we simply map this
Expand All @@ -287,9 +309,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
CanonicalVarValues { var_values }
}

#[instrument(level = "debug", skip(self, param_env), ret)]
#[instrument(level = "debug", skip(infcx, param_env), ret)]
fn unify_query_var_values(
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

&self,
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &[ty::GenericArg<'tcx>],
var_values: CanonicalVarValues<'tcx>,
Expand All @@ -298,7 +320,18 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

let mut nested_goals = vec![];
for (&orig, response) in iter::zip(original_values, var_values.var_values) {
nested_goals.extend(self.eq_and_get_goals(param_env, orig, response)?);
nested_goals.extend(
infcx
.at(&ObligationCause::dummy(), param_env)
.eq(DefineOpaqueTypes::No, orig, response)
.map(|InferOk { value: (), obligations }| {
obligations.into_iter().map(|o| Goal::from(o))
})
.map_err(|e| {
debug!(?e, "failed to equate");
NoSolution
})?,
);
}

Ok(nested_goals)
Expand Down Expand Up @@ -403,3 +436,35 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for EagerResolver<'_, 'tcx> {
}
}
}

impl<'tcx> inspect::ProofTreeBuilder<'tcx> {
pub fn make_canonical_state<T: TypeFoldable<TyCtxt<'tcx>>>(
ecx: &EvalCtxt<'_, 'tcx>,
data: T,
) -> inspect::CanonicalState<'tcx, T> {
let state = inspect::State { var_values: ecx.var_values, data };
let state = state.fold_with(&mut EagerResolver { infcx: ecx.infcx });
Canonicalizer::canonicalize(
ecx.infcx,
CanonicalizeMode::Response { max_input_universe: ecx.max_input_universe },
&mut vec![],
state,
)
}

pub fn instantiate_canonical_state<T: TypeFoldable<TyCtxt<'tcx>>>(
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &[ty::GenericArg<'tcx>],
state: inspect::CanonicalState<'tcx, T>,
) -> Result<(Vec<Goal<'tcx, ty::Predicate<'tcx>>>, T), NoSolution> {
let substitution =
EvalCtxt::compute_query_response_substitution(infcx, original_values, &state);

let inspect::State { var_values, data } = state.substitute(infcx.tcx, &substitution);

let nested_goals =
EvalCtxt::unify_query_var_values(infcx, param_env, original_values, var_values)?;
Ok((nested_goals, data))
}
}
Loading