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

alias-relate: add fast reject optimization #124852

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
113 changes: 112 additions & 1 deletion compiler/rustc_trait_selection/src/solve/alias_relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
//! relate them structurally.

use super::EvalCtxt;
use rustc_data_structures::fx::FxHashSet;
use rustc_infer::infer::InferCtxt;
use rustc_middle::traits::query::NoSolution;
use rustc_middle::traits::solve::{Certainty, Goal, QueryResult};
use rustc_middle::ty;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::ty::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor};

impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
#[instrument(level = "trace", skip(self), ret)]
Expand All @@ -29,6 +32,12 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
let tcx = self.tcx();
let Goal { param_env, predicate: (lhs, rhs, direction) } = goal;

if self.alias_cannot_name_placeholder_in_rigid(param_env, lhs, rhs)
|| self.alias_cannot_name_placeholder_in_rigid(param_env, rhs, lhs)
{
return Err(NoSolution);
}

// Structurally normalize the lhs.
let lhs = if let Some(alias) = lhs.to_alias_term() {
let term = self.next_term_infer_of_kind(lhs);
Expand Down Expand Up @@ -84,3 +93,105 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
}
}
}

enum IgnoreAliases {
Yes,
No,
}

impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
/// In case a rigid term refers to a placeholder which is not referenced by the
/// alias, the alias cannot be normalized to that rigid term unless it contains
/// either inference variables or these placeholders are referenced in a term
/// of a `Projection`-clause in the environment.
fn alias_cannot_name_placeholder_in_rigid(
&mut self,
param_env: ty::ParamEnv<'tcx>,
rigid_term: ty::Term<'tcx>,
alias: ty::Term<'tcx>,
) -> bool {
// Check that the rigid term is actually rigid.
if rigid_term.to_alias_term().is_some() || alias.to_alias_term().is_none() {
return false;
}

// If the alias has any type or const inference variables,
// do not try to apply the fast path as these inference variables
// may resolve to something containing placeholders.
if alias.has_non_region_infer() {
return false;
}

let mut referenced_placeholders =
Copy link
Member

Choose a reason for hiding this comment

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

Why IgnoreAliases::Yes?

Copy link
Member

Choose a reason for hiding this comment

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

Feels like this fast reject could be quickly side-stepped w/ something like Mirror<!A> alias-relate Mirror<!B>

Copy link
Member

Choose a reason for hiding this comment

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

Update: It occurred to me last night while I was sleeping that we ignore aliases here b/c e.g. Alias<!A> could normalize to sth not mentioning !A.

self.collect_placeholders_in_term(rigid_term, IgnoreAliases::Yes);
for clause in param_env.caller_bounds() {
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to explain in detail what this is searching for. Specifically, we disqualify any of our rigid term's placeholders if they show up in projection goals, since that may assist in normalizing.

e.g. if we have

rigid = W<!A>
alias = Alias<!B>
param-env = [Alias<!B> normalizes-to W<!A>, ...]

then we exclude !A from our rigid placeholder list, right?

I think this may also be worth mentioning that this doesn't really care about how the placeholders show up in the term of the projection predicate, since this is really just a quick heuristic.

match clause.kind().skip_binder() {
ty::ClauseKind::Projection(ty::ProjectionPredicate { term, .. }) => {
if term.has_non_region_infer() {
return false;
}

let env_term_placeholders =
self.collect_placeholders_in_term(term, IgnoreAliases::No);
#[allow(rustc::potential_query_instability)]
referenced_placeholders.retain(|p| !env_term_placeholders.contains(p));
}
ty::ClauseKind::Trait(_)
| ty::ClauseKind::TypeOutlives(_)
| ty::ClauseKind::RegionOutlives(_)
| ty::ClauseKind::ConstArgHasType(..)
| ty::ClauseKind::WellFormed(_)
| ty::ClauseKind::ConstEvaluatable(_) => continue,
}
}

if referenced_placeholders.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

Probably a somewhat redundant note, but maybe just say "empty set is a subset of everything, so no need to compute placeholders of alias"

return false;
}

let alias_placeholders = self.collect_placeholders_in_term(alias, IgnoreAliases::No);
// If the rigid term references a placeholder not mentioned by the alias,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// If the rigid term references a placeholder not mentioned by the alias,
// If the rigid term references a placeholder not mentioned by the alias, and isn't potentially reachable by a projection-predicate,

or something, to explain the way that the projection predicate search above influences this search

// they can never unify.
!referenced_placeholders.is_subset(&alias_placeholders)
}

fn collect_placeholders_in_term(
&mut self,
term: ty::Term<'tcx>,
ignore_aliases: IgnoreAliases,
) -> FxHashSet<ty::Term<'tcx>> {
// Fast path to avoid walking the term.
if !term.has_placeholders() {
return Default::default();
}

struct PlaceholderCollector<'tcx> {
ignore_aliases: IgnoreAliases,
placeholders: FxHashSet<ty::Term<'tcx>>,
}
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for PlaceholderCollector<'tcx> {
type Result = ();

fn visit_ty(&mut self, t: Ty<'tcx>) {
match t.kind() {
ty::Placeholder(_) => drop(self.placeholders.insert(t.into())),
ty::Alias(..) if matches!(self.ignore_aliases, IgnoreAliases::Yes) => {}
_ => t.super_visit_with(self),
}
}

fn visit_const(&mut self, ct: ty::Const<'tcx>) {
match ct.kind() {
ty::ConstKind::Placeholder(_) => drop(self.placeholders.insert(ct.into())),
ty::ConstKind::Unevaluated(_) | ty::ConstKind::Expr(_)
if matches!(self.ignore_aliases, IgnoreAliases::Yes) => {}
_ => ct.super_visit_with(self),
}
}
}

let mut visitor = PlaceholderCollector { ignore_aliases, placeholders: Default::default() };
term.visit_with(&mut visitor);
visitor.placeholders
}
}
Loading