Skip to content

skip proving the trait goal if possible in NormalizesTo goal #149533

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
+264 −200
Draft

skip proving the trait goal if possible in NormalizesTo goal #149533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
128 changes: 18 additions & 110 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use rustc_type_ir::{
};
use tracing::{debug, instrument};

use super::trait_goals::TraitGoalProvenVia;
use super::{has_only_region_constraints, inspect};
use crate::delegate::SolverDelegate;
use crate::solve::inspect::ProbeKind;
Expand Down Expand Up @@ -361,13 +360,15 @@ pub(super) enum AssembleCandidatesFrom {
/// user-written and built-in impls. We only expect `ParamEnv` and `AliasBound`
/// candidates to be assembled.
EnvAndBounds,
Impl,
}

impl AssembleCandidatesFrom {
fn should_assemble_impl_candidates(&self) -> bool {
match self {
AssembleCandidatesFrom::All => true,
AssembleCandidatesFrom::EnvAndBounds => false,
AssembleCandidatesFrom::Impl => true,
}
}
}
Expand Down Expand Up @@ -424,11 +425,14 @@ where
return (candidates, failed_candidate_info);
}

self.assemble_alias_bound_candidates(goal, &mut candidates);
self.assemble_param_env_candidates(goal, &mut candidates, &mut failed_candidate_info);

match assemble_from {
AssembleCandidatesFrom::All => {
self.assemble_alias_bound_candidates(goal, &mut candidates);
self.assemble_param_env_candidates(
goal,
&mut candidates,
&mut failed_candidate_info,
);
self.assemble_builtin_impl_candidates(goal, &mut candidates);
// For performance we only assemble impls if there are no candidates
// which would shadow them. This is necessary to avoid hangs in rayon,
Expand All @@ -455,6 +459,12 @@ where
}
}
AssembleCandidatesFrom::EnvAndBounds => {
self.assemble_alias_bound_candidates(goal, &mut candidates);
self.assemble_param_env_candidates(
goal,
&mut candidates,
&mut failed_candidate_info,
);
// This is somewhat inconsistent and may make #57893 slightly easier to exploit.
// However, it matches the behavior of the old solver. See
// `tests/ui/traits/next-solver/normalization-shadowing/use_object_if_empty_env.rs`.
Expand All @@ -464,6 +474,10 @@ where
self.assemble_object_bound_candidates(goal, &mut candidates);
}
}
AssembleCandidatesFrom::Impl => {
self.assemble_builtin_impl_candidates(goal, &mut candidates);
self.assemble_impl_candidates(goal, &mut candidates);
}
}

(candidates, failed_candidate_info)
Expand Down Expand Up @@ -1095,112 +1109,6 @@ where
}
}

/// Assemble and merge candidates for goals which are related to an underlying trait
/// goal. Right now, this is normalizes-to and host effect goals.
///
/// We sadly can't simply take all possible candidates for normalization goals
/// and check whether they result in the same constraints. We want to make sure
/// that trying to normalize an alias doesn't result in constraints which aren't
/// otherwise required.
///
/// Most notably, when proving a trait goal by via a where-bound, we should not
/// normalize via impls which have stricter region constraints than the where-bound:
///
/// ```rust
/// trait Trait<'a> {
/// type Assoc;
/// }
///
/// impl<'a, T: 'a> Trait<'a> for T {
/// type Assoc = u32;
/// }
///
/// fn with_bound<'a, T: Trait<'a>>(_value: T::Assoc) {}
/// ```
///
/// The where-bound of `with_bound` doesn't specify the associated type, so we would
/// only be able to normalize `<T as Trait<'a>>::Assoc` by using the impl. This impl
/// adds a `T: 'a` bound however, which would result in a region error. Given that the
/// user explicitly wrote that `T: Trait<'a>` holds, this is undesirable and we instead
/// treat the alias as rigid.
///
/// See trait-system-refactor-initiative#124 for more details.
#[instrument(level = "debug", skip_all, fields(proven_via, goal), ret)]
pub(super) fn assemble_and_merge_candidates<G: GoalKind<D>>(
&mut self,
proven_via: Option<TraitGoalProvenVia>,
goal: Goal<I, G>,
inject_forced_ambiguity_candidate: impl FnOnce(&mut EvalCtxt<'_, D>) -> Option<QueryResult<I>>,
inject_normalize_to_rigid_candidate: impl FnOnce(&mut EvalCtxt<'_, D>) -> QueryResult<I>,
) -> QueryResult<I> {
let Some(proven_via) = proven_via else {
// We don't care about overflow. If proving the trait goal overflowed, then
// it's enough to report an overflow error for that, we don't also have to
// overflow during normalization.
//
// We use `forced_ambiguity` here over `make_ambiguous_response_no_constraints`
// because the former will also record a built-in candidate in the inspector.
return self.forced_ambiguity(MaybeCause::Ambiguity).map(|cand| cand.result);
};

match proven_via {
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
// Even when a trait bound has been proven using a where-bound, we
// still need to consider alias-bounds for normalization, see
// `tests/ui/next-solver/alias-bound-shadowed-by-env.rs`.
let (mut candidates, _) = self
.assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::EnvAndBounds);
debug!(?candidates);

// If the trait goal has been proven by using the environment, we want to treat
// aliases as rigid if there are no applicable projection bounds in the environment.
if candidates.is_empty() {
return inject_normalize_to_rigid_candidate(self);
}

// If we're normalizing an GAT, we bail if using a where-bound would constrain
// its generic arguments.
if let Some(result) = inject_forced_ambiguity_candidate(self) {
return result;
}

// We still need to prefer where-bounds over alias-bounds however.
// See `tests/ui/winnowing/norm-where-bound-gt-alias-bound.rs`.
if candidates.iter().any(|c| matches!(c.source, CandidateSource::ParamEnv(_))) {
candidates.retain(|c| matches!(c.source, CandidateSource::ParamEnv(_)));
}

if let Some((response, _)) = self.try_merge_candidates(&candidates) {
Ok(response)
} else {
self.flounder(&candidates)
}
}
TraitGoalProvenVia::Misc => {
let (mut candidates, _) =
self.assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::All);

// Prefer "orphaned" param-env normalization predicates, which are used
// (for example, and ideally only) when proving item bounds for an impl.
if candidates.iter().any(|c| matches!(c.source, CandidateSource::ParamEnv(_))) {
candidates.retain(|c| matches!(c.source, CandidateSource::ParamEnv(_)));
}

// We drop specialized impls to allow normalization via a final impl here. In case
// the specializing impl has different inference constraints from the specialized
// impl, proving the trait goal is already ambiguous, so we never get here. This
// means we can just ignore inference constraints and don't have to special-case
// constraining the normalized-to `term`.
self.filter_specialized_impls(AllowInferenceConstraints::Yes, &mut candidates);
if let Some((response, _)) = self.try_merge_candidates(&candidates) {
Ok(response)
} else {
self.flounder(&candidates)
}
}
}
}

/// Compute whether a param-env assumption is global or non-global after normalizing it.
///
/// This is necessary because, for example, given:
Expand Down
65 changes: 61 additions & 4 deletions compiler/rustc_next_trait_solver/src/solve/effect_goals.rs
View file
Open in desktop
Copy link
Contributor Author

@adwinwhite adwinwhite Dec 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure whether candidate preference matters to effect goal.

Copy link
Contributor

@lcnr lcnr Dec 2, 2025

Choose a reason for hiding this comment

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

it's whatever :> please add a FIXME(const_traits) to assemble_and_merge_candidates that this was copied from the old projection normalization behavior and might not be what we actually want here

Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ use rustc_type_ir::lang_items::SolverTraitLangItem;
use rustc_type_ir::solve::inspect::ProbeKind;
use rustc_type_ir::solve::{AliasBoundKind, SizedTraitKind};
use rustc_type_ir::{self as ty, Interner, TypingMode, elaborate};
use tracing::instrument;
use tracing::{debug, instrument};

use super::assembly::{Candidate, structural_traits};
use crate::delegate::SolverDelegate;
use crate::solve::assembly::{AllowInferenceConstraints, AssembleCandidatesFrom};
use crate::solve::trait_goals::TraitGoalProvenVia;
use crate::solve::{
BuiltinImplSource, CandidateSource, Certainty, EvalCtxt, Goal, GoalSource, NoSolution,
QueryResult, assembly,
BuiltinImplSource, CandidateSource, Certainty, EvalCtxt, Goal, GoalSource, MaybeCause,
NoSolution, QueryResult, assembly,
};

impl<D, I> assembly::GoalKind<D> for ty::HostEffectPredicate<I>
Expand Down Expand Up @@ -446,6 +448,61 @@ where
goal.with(ecx.cx(), goal.predicate.trait_ref);
ecx.compute_trait_goal(trait_goal)
})?;
self.assemble_and_merge_candidates(proven_via, goal, |_ecx| None, |_ecx| Err(NoSolution))
self.assemble_and_merge_candidates(proven_via, goal)
}

#[instrument(level = "debug", skip(self), ret)]
fn assemble_and_merge_candidates(
&mut self,
proven_via: Option<TraitGoalProvenVia>,
goal: Goal<I, ty::HostEffectPredicate<I>>,
) -> QueryResult<I> {
let Some(proven_via) = proven_via else {
// We don't care about overflow. If proving the trait goal overflowed, then
// it's enough to report an overflow error for that, we don't also have to
// overflow during normalization.
Comment on lines +461 to +463
Copy link
Contributor

@lcnr lcnr Dec 2, 2025

Choose a reason for hiding this comment

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

Suggested change
// We don't care about overflow. If proving the trait goal overflowed, then
// it's enough to report an overflow error for that, we don't also have to
// overflow during normalization.
// We don't care about overflow. If proving the trait goal overflowed, then
// it's enough to report an overflow error for that, we don't also have to
// overflow for the const bound.

or sth

//
// We use `forced_ambiguity` here over `make_ambiguous_response_no_constraints`
// because the former will also record a built-in candidate in the inspector.
return self.forced_ambiguity(MaybeCause::Ambiguity).map(|cand| cand.result);
};

match proven_via {
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
let (mut candidates, _) = self
.assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::EnvAndBounds);
debug!(?candidates);

if candidates.iter().any(|c| matches!(c.source, CandidateSource::ParamEnv(_))) {
candidates.retain(|c| matches!(c.source, CandidateSource::ParamEnv(_)));
}

if let Some((response, _)) = self.try_merge_candidates(&candidates) {
Ok(response)
} else {
self.flounder(&candidates)
}
}
TraitGoalProvenVia::Misc => {
let (mut candidates, _) =
self.assemble_and_evaluate_candidates(goal, AssembleCandidatesFrom::All);

if candidates.iter().any(|c| matches!(c.source, CandidateSource::ParamEnv(_))) {
candidates.retain(|c| matches!(c.source, CandidateSource::ParamEnv(_)));
}

// We drop specialized impls to allow normalization via a final impl here. In case
// the specializing impl has different inference constraints from the specialized
// impl, proving the trait goal is already ambiguous, so we never get here. This
// means we can just ignore inference constraints and don't have to special-case
// constraining the normalized-to `term`.
self.filter_specialized_impls(AllowInferenceConstraints::Yes, &mut candidates);
if let Some((response, _)) = self.try_merge_candidates(&candidates) {
Ok(response)
} else {
self.flounder(&candidates)
}
}
}
}
}
52 changes: 2 additions & 50 deletions compiler/rustc_next_trait_solver/src/solve/normalizes_to/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod anon_const;
mod free_alias;
mod inherent;
mod opaque_types;
mod projection;

use rustc_type_ir::fast_reject::DeepRejectCtxt;
use rustc_type_ir::inherent::*;
Expand All @@ -13,7 +14,6 @@ use tracing::instrument;
use crate::delegate::SolverDelegate;
use crate::solve::assembly::structural_traits::{self, AsyncCallableRelevantTypes};
use crate::solve::assembly::{self, Candidate};
use crate::solve::inspect::ProbeKind;
use crate::solve::{
BuiltinImplSource, CandidateSource, Certainty, EvalCtxt, Goal, GoalSource, MaybeCause,
NoSolution, QueryResult,
Expand All @@ -33,55 +33,7 @@ where
let cx = self.cx();
match goal.predicate.alias.kind(cx) {
ty::AliasTermKind::ProjectionTy | ty::AliasTermKind::ProjectionConst => {
let trait_ref = goal.predicate.alias.trait_ref(cx);
let (_, proven_via) =
self.probe(|_| ProbeKind::ShadowedEnvProbing).enter(|ecx| {
let trait_goal: Goal<I, ty::TraitPredicate<I>> = goal.with(cx, trait_ref);
ecx.compute_trait_goal(trait_goal)
})?;
self.assemble_and_merge_candidates(
proven_via,
goal,
|ecx| {
// FIXME(generic_associated_types): Addresses aggressive inference in #92917.
//
// If this type is a GAT with currently unconstrained arguments, we do not
// want to normalize it via a candidate which only applies for a specific
// instantiation. We could otherwise keep the GAT as rigid and succeed this way.
// See tests/ui/generic-associated-types/no-incomplete-gat-arg-inference.rs.
//
// This only avoids normalization if a GAT argument is fully unconstrained.
// This is quite arbitrary but fixing it causes some ambiguity, see #125196.
for arg in goal.predicate.alias.own_args(cx).iter() {
let Some(term) = arg.as_term() else {
continue;
};
match ecx.structurally_normalize_term(goal.param_env, term) {
Ok(term) => {
if term.is_infer() {
return Some(
ecx.evaluate_added_goals_and_make_canonical_response(
Certainty::AMBIGUOUS,
),
);
}
}
Err(NoSolution) => return Some(Err(NoSolution)),
}
}

None
},
|ecx| {
ecx.probe(|&result| ProbeKind::RigidAlias { result }).enter(|this| {
this.structurally_instantiate_normalizes_to_term(
goal,
goal.predicate.alias,
);
this.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
})
},
)
self.normalize_trait_associated_term(goal)
}
ty::AliasTermKind::InherentTy | ty::AliasTermKind::InherentConst => {
self.normalize_inherent_associated_term(goal)
Expand Down
Loading
Loading