bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #129896 - lcnr:bail-on-unknowable, r=jackh726

Browse source 
do not attempt to prove unknowable goals

In case a goal is unknowable, we previously still checked all other possible ways to prove this goal, even though its final result is already guaranteed to be ambiguous. By ignoring all other candidates in that case we can avoid a lot of unnecessary work, fixing the performance regression in typenum found in #121848.

This is already the behavior in the old solver. This could in theory cause future-compatability issues as considering fewer goals unknowable may end up causing performance regressions/hangs. I am quite confident that this will not be an issue.

r? ``@compiler-errors``
This commit is contained in:
Matthias Krüger 2024-09-03 19:13:26 +02:00 • committed by GitHub
commit 485fd3815c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 100 additions and 108 deletions
Download patch file Download diff file Expand all files Collapse all files

136
compiler/rustc_trait_selection/src/traits/coherence.rs
View file

@ -29,6 +29,7 @@ use crate::infer::outlives::env::OutlivesEnvironment;
use crate::infer::InferOk;
use crate::solve::inspect::{InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor};
use crate::solve::{deeply_normalize_for_diagnostics, inspect};
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::{
util, FulfillmentErrorCode, NormalizeExt, Obligation, ObligationCause, PredicateObligation,
@ -624,14 +625,13 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a, 'tcx> {
// at ambiguous goals, as for others the coherence unknowable candidate
// was irrelevant.
match goal.result() {
Ok(Certainty::Maybe(_)) => {}
Ok(Certainty::Yes) | Err(NoSolution) => return,
Ok(Certainty::Maybe(_)) => {}
}
let Goal { param_env, predicate } = goal.goal();
// For bound predicates we simply call `infcx.enter_forall`
// and then prove the resulting predicate as a nested goal.
let Goal { param_env, predicate } = goal.goal();
let trait_ref = match predicate.kind().no_bound_vars() {
Some(ty::PredicateKind::Clause(ty::ClauseKind::Trait(tr))) => tr.trait_ref,
Some(ty::PredicateKind::Clause(ty::ClauseKind::Projection(proj)))
@ -645,7 +645,11 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a, 'tcx> {
_ => return,
};
// Add ambiguity causes for reservation impls.
if trait_ref.references_error() {
return;
}
let mut candidates = goal.candidates();
for cand in goal.candidates() {
if let inspect::ProbeKind::TraitCandidate {
source: CandidateSource::Impl(def_id),
@ -664,78 +668,68 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a, 'tcx> {
}
}
// Add ambiguity causes for unknowable goals.
let mut ambiguity_cause = None;
for cand in goal.candidates() {
if let inspect::ProbeKind::TraitCandidate {
source: CandidateSource::CoherenceUnknowable,
result: Ok(_),
} = cand.kind()
{
let lazily_normalize_ty = |mut ty: Ty<'tcx>| {
if matches!(ty.kind(), ty::Alias(..)) {
let ocx = ObligationCtxt::new(infcx);
ty = ocx
.structurally_normalize(&ObligationCause::dummy(), param_env, ty)
.map_err(|_| ())?;
if !ocx.select_where_possible().is_empty() {
return Err(());
}
}
Ok(ty)
};
// We also look for unknowable candidates. In case a goal is unknowable, there's
// always exactly 1 candidate.
let Some(cand) = candidates.pop() else {
return;
};
infcx.probe(|_| {
match trait_ref_is_knowable(infcx, trait_ref, lazily_normalize_ty) {
Err(()) => {}
Ok(Ok(())) => warn!("expected an unknowable trait ref: {trait_ref:?}"),
Ok(Err(conflict)) => {
if !trait_ref.references_error() {
// Normalize the trait ref for diagnostics, ignoring any errors if this fails.
let trait_ref =
deeply_normalize_for_diagnostics(infcx, param_env, trait_ref);
let inspect::ProbeKind::TraitCandidate {
source: CandidateSource::CoherenceUnknowable,
result: Ok(_),
} = cand.kind()
else {
return;
};
let self_ty = trait_ref.self_ty();
let self_ty = self_ty.has_concrete_skeleton().then(|| self_ty);
ambiguity_cause = Some(match conflict {
Conflict::Upstream => {
IntercrateAmbiguityCause::UpstreamCrateUpdate {
trait_ref,
self_ty,
}
}
Conflict::Downstream => {
IntercrateAmbiguityCause::DownstreamCrate {
trait_ref,
self_ty,
}
}
});
}
}
}
})
} else {
match cand.result() {
// We only add an ambiguity cause if the goal would otherwise
// result in an error.
//
// FIXME: While this matches the behavior of the
// old solver, it is not the only way in which the unknowable
// candidates *weaken* coherence, they can also force otherwise
// successful normalization to be ambiguous.
Ok(Certainty::Maybe(_) | Certainty::Yes) => {
ambiguity_cause = None;
break;
}
Err(NoSolution) => continue,
let lazily_normalize_ty = |mut ty: Ty<'tcx>| {
if matches!(ty.kind(), ty::Alias(..)) {
let ocx = ObligationCtxt::new(infcx);
ty = ocx
.structurally_normalize(&ObligationCause::dummy(), param_env, ty)
.map_err(|_| ())?;
if !ocx.select_where_possible().is_empty() {
return Err(());
}
}
}
Ok(ty)
};
if let Some(ambiguity_cause) = ambiguity_cause {
self.causes.insert(ambiguity_cause);
}
infcx.probe(|_| {
let conflict = match trait_ref_is_knowable(infcx, trait_ref, lazily_normalize_ty) {
Err(()) => return,
Ok(Ok(())) => {
warn!("expected an unknowable trait ref: {trait_ref:?}");
return;
}
Ok(Err(conflict)) => conflict,
};
// It is only relevant that a goal is unknowable if it would have otherwise
// failed.
let non_intercrate_infcx = infcx.fork_with_intercrate(false);
if non_intercrate_infcx.predicate_may_hold(&Obligation::new(
infcx.tcx,
ObligationCause::dummy(),
param_env,
predicate,
)) {
return;
}
// Normalize the trait ref for diagnostics, ignoring any errors if this fails.
let trait_ref = deeply_normalize_for_diagnostics(infcx, param_env, trait_ref);
let self_ty = trait_ref.self_ty();
let self_ty = self_ty.has_concrete_skeleton().then(|| self_ty);
self.causes.insert(match conflict {
Conflict::Upstream => {
IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_ref, self_ty }
}
Conflict::Downstream => {
IntercrateAmbiguityCause::DownstreamCrate { trait_ref, self_ty }
}
});
});
}
}