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

remove some potentially unnecessary incompleteness #113445

Closed
wants to merge 1 commit into from
Closed
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
32 changes: 23 additions & 9 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use super::search_graph::OverflowHandler;
use super::{EvalCtxt, SolverMode};
use crate::solve::CanonicalResponseExt;
use crate::traits::coherence;
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -84,7 +85,7 @@ pub(super) enum CandidateSource {
/// let _y = x.clone();
/// }
/// ```
AliasBound,
AliasBound(ty::AliasKind),
}

/// Records additional information about what kind of built-in impl this is.
Expand Down Expand Up @@ -510,7 +511,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
goal: Goal<'tcx, G>,
candidates: &mut Vec<Candidate<'tcx>>,
) {
let alias_ty = match goal.predicate.self_ty().kind() {
let (kind, alias_ty) = match goal.predicate.self_ty().kind() {
ty::Bool
| ty::Char
| ty::Int(_)
Expand Down Expand Up @@ -541,14 +542,14 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
ty::Infer(ty::TyVar(_) | ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_))
| ty::Bound(..) => bug!("unexpected self type for `{goal:?}`"),
// Excluding IATs and type aliases here as they don't have meaningful item bounds.
ty::Alias(ty::Projection | ty::Opaque, alias_ty) => alias_ty,
&ty::Alias(kind @ (ty::Projection | ty::Opaque), alias_ty) => (kind, alias_ty),
};

for assumption in self.tcx().item_bounds(alias_ty.def_id).subst(self.tcx(), alias_ty.substs)
{
match G::consider_alias_bound_candidate(self, goal, assumption) {
Ok(result) => {
candidates.push(Candidate { source: CandidateSource::AliasBound, result })
candidates.push(Candidate { source: CandidateSource::AliasBound(kind), result })
}
Err(NoSolution) => (),
}
Expand Down Expand Up @@ -772,23 +773,36 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
// Doing so is incomplete and would therefore be unsound during coherence.
match self.solver_mode() {
SolverMode::Coherence => (),
// Prioritize `ParamEnv` candidates only if they do not guide inference.
// Incompletely prioritize candidates from the environment-
//
// This is still incomplete as we may add incorrect region bounds.
// See https://github.com/rust-lang/trait-system-refactor-initiative/issues/45
// for more details.
SolverMode::Normal => {
// Prefer candidates from the environment if they only add region constraints.
let param_env_responses = candidates
.iter()
.filter(|c| {
matches!(
c.source,
CandidateSource::ParamEnv(_) | CandidateSource::AliasBound
CandidateSource::ParamEnv(_) | CandidateSource::AliasBound(_)
)
})
.map(|c| c.result)
.collect::<Vec<_>>();
if let Some(result) = self.try_merge_responses(&param_env_responses) {
// We strongly prefer alias and param-env bounds here, even if they affect inference.
// See https://github.com/rust-lang/trait-system-refactor-initiative/issues/11.
if result.has_only_region_constraints() {
return Ok(result);
}
}

// Prefer alias bound candidates for opaque types.
let opaque_alias_bounds = candidates
.iter()
.filter(|c| matches!(c.source, CandidateSource::AliasBound(ty::Opaque)))
.map(|c| c.result)
.collect::<Vec<_>>();

if let Some(result) = self.try_merge_responses(&opaque_alias_bounds) {
return Ok(result);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<'tcx> InferCtxtSelectExt<'tcx> for InferCtxt<'tcx> {

// It's fine not to do anything to rematch these, since there are no
// nested obligations.
(Certainty::Yes, CandidateSource::ParamEnv(_) | CandidateSource::AliasBound) => {
(Certainty::Yes, CandidateSource::ParamEnv(_) | CandidateSource::AliasBound(_)) => {
Ok(Some(ImplSource::Param(nested_obligations, ty::BoundConstness::NotConst)))
}

Expand Down
Loading