Skip to content

only return nested goals for Certainty::Yes #140402

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 2 commits into from
Apr 29, 2025
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
52 changes: 31 additions & 21 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,19 @@ where
/// the values inferred while solving the instantiated goal.
/// - `external_constraints`: additional constraints which aren't expressible
/// using simple unification of inference variables.
///
/// This takes the `shallow_certainty` which represents whether we're confident
/// that the final result of the current goal only depends on the nested goals.
///
/// In case this is `Certainy::Maybe`, there may still be additional nested goals
/// or inference constraints required for this candidate to be hold. The candidate
/// always requires all already added constraints and nested goals.
#[instrument(level = "trace", skip(self), ret)]
pub(in crate::solve) fn evaluate_added_goals_and_make_canonical_response(
&mut self,
certainty: Certainty,
shallow_certainty: Certainty,
) -> QueryResult<I> {
self.inspect.make_canonical_response(certainty);
self.inspect.make_canonical_response(shallow_certainty);

let goals_certainty = self.try_evaluate_added_goals()?;
assert_eq!(
Expand All @@ -103,26 +110,29 @@ where
NoSolution
})?;

// When normalizing, we've replaced the expected term with an unconstrained
// inference variable. This means that we dropped information which could
// have been important. We handle this by instead returning the nested goals
// to the caller, where they are then handled.
//
// As we return all ambiguous nested goals, we can ignore the certainty returned
// by `try_evaluate_added_goals()`.
let (certainty, normalization_nested_goals) = match self.current_goal_kind {
CurrentGoalKind::NormalizesTo => {
let goals = std::mem::take(&mut self.nested_goals);
if goals.is_empty() {
assert!(matches!(goals_certainty, Certainty::Yes));
let (certainty, normalization_nested_goals) =
match (self.current_goal_kind, shallow_certainty) {
// When normalizing, we've replaced the expected term with an unconstrained
// inference variable. This means that we dropped information which could
// have been important. We handle this by instead returning the nested goals
// to the caller, where they are then handled. We only do so if we do not
// need to recompute the `NormalizesTo` goal afterwards to avoid repeatedly
// uplifting its nested goals. This is the case if the `shallow_certainty` is
// `Certainty::Yes`.
(CurrentGoalKind::NormalizesTo, Certainty::Yes) => {
let goals = std::mem::take(&mut self.nested_goals);
// As we return all ambiguous nested goals, we can ignore the certainty
// returned by `self.try_evaluate_added_goals()`.
if goals.is_empty() {
assert!(matches!(goals_certainty, Certainty::Yes));
}
(Certainty::Yes, NestedNormalizationGoals(goals))
}
(certainty, NestedNormalizationGoals(goals))
}
CurrentGoalKind::Misc | CurrentGoalKind::CoinductiveTrait => {
let certainty = certainty.unify_with(goals_certainty);
(certainty, NestedNormalizationGoals::empty())
}
};
_ => {
let certainty = shallow_certainty.unify_with(goals_certainty);
(certainty, NestedNormalizationGoals::empty())
}
};

if let Certainty::Maybe(cause @ MaybeCause::Overflow { .. }) = certainty {
// If we have overflow, it's probable that we're substituting a type
Expand Down
38 changes: 28 additions & 10 deletions compiler/rustc_next_trait_solver/src/solve/normalizes_to/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod weak_types;
use rustc_type_ir::fast_reject::DeepRejectCtxt;
use rustc_type_ir::inherent::*;
use rustc_type_ir::lang_items::TraitSolverLangItem;
use rustc_type_ir::{self as ty, Interner, NormalizesTo, Upcast as _};
use rustc_type_ir::{self as ty, Interner, NormalizesTo, PredicateKind, Upcast as _};
use tracing::instrument;

use crate::delegate::SolverDelegate;
Expand Down Expand Up @@ -221,13 +221,21 @@ where
Ok(Some(target_item_def_id)) => target_item_def_id,
Ok(None) => {
match ecx.typing_mode() {
// In case the associated item is hidden due to specialization, we have to
// return ambiguity this would otherwise be incomplete, resulting in
// unsoundness during coherence (#105782).
// In case the associated item is hidden due to specialization,
// normalizing this associated item is always ambiguous. Treating
// the associated item as rigid would be incomplete and allow for
// overlapping impls, see #105782.
//
// As this ambiguity is unavoidable we emit a nested ambiguous
// goal instead of using `Certainty::AMBIGUOUS`. This allows us to
// return the nested goals to the parent `AliasRelate` goal. This
// would be relevant if any of the nested goals refer to the `term`.
// This is not the case here and we only prefer adding an ambiguous
// nested goal for consistency.
ty::TypingMode::Coherence => {
return ecx.evaluate_added_goals_and_make_canonical_response(
Certainty::AMBIGUOUS,
);
ecx.add_goal(GoalSource::Misc, goal.with(cx, PredicateKind::Ambiguous));
return ecx
.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
}
// Outside of coherence, we treat the associated item as rigid instead.
ty::TypingMode::Analysis { .. }
Expand All @@ -254,10 +262,20 @@ where
// treat it as rigid.
if cx.impl_self_is_guaranteed_unsized(impl_def_id) {
match ecx.typing_mode() {
// Trying to normalize such associated items is always ambiguous
// during coherence to avoid cyclic reasoning. See the example in
// tests/ui/traits/trivial-unsized-projection-in-coherence.rs.
//
// As this ambiguity is unavoidable we emit a nested ambiguous
// goal instead of using `Certainty::AMBIGUOUS`. This allows us to
// return the nested goals to the parent `AliasRelate` goal. This
// would be relevant if any of the nested goals refer to the `term`.
// This is not the case here and we only prefer adding an ambiguous
// nested goal for consistency.
ty::TypingMode::Coherence => {
return ecx.evaluate_added_goals_and_make_canonical_response(
Certainty::AMBIGUOUS,
);
ecx.add_goal(GoalSource::Misc, goal.with(cx, PredicateKind::Ambiguous));
return ecx
.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
}
ty::TypingMode::Analysis { .. }
| ty::TypingMode::Borrowck { .. }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use rustc_index::bit_set::GrowableBitSet;
use rustc_type_ir::inherent::*;
use rustc_type_ir::solve::GoalSource;
use rustc_type_ir::{self as ty, Interner, TypingMode, fold_regions};

use crate::delegate::SolverDelegate;
Expand Down Expand Up @@ -31,7 +32,12 @@ where
goal.param_env,
expected,
);
self.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
// Trying to normalize an opaque type during coherence is always ambiguous.
// We add a nested ambiguous goal here instead of using `Certainty::AMBIGUOUS`.
// This allows us to return the nested goals to the parent `AliasRelate` goal.
// This can then allow nested goals to fail after we've constrained the `term`.
self.add_goal(GoalSource::Misc, goal.with(cx, ty::PredicateKind::Ambiguous));
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
}
TypingMode::Analysis { defining_opaque_types_and_generators } => {
let Some(def_id) = opaque_ty
Expand Down
Loading