Skip to content

Commit

Permalink
Auto merge of #48296 - ishitatsuyuki:exp-unblow, r=<try>
Browse files Browse the repository at this point in the history
Fix exponential projection complexity on nested types

This implements solution 1 from #38528 (comment).

The code quality is currently extremely poor, but we can improve them during review.

Blocking issues:

- we probably don't want a quadratic deduplication for obligations.
- is there an alternative to deduplication?

Based on #48315.

Needs changelog. Noticable improvement on compile time is expected.

Fix #38528
Close #39684
Close #43757
  • Loading branch information
bors committed Feb 18, 2018
2 parents 1ad094d + 4a40c55 commit 4ac2425
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 119 deletions.
6 changes: 3 additions & 3 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ pub type SelectionResult<'tcx, T> = Result<Option<T>, SelectionError<'tcx>>;
/// ### The type parameter `N`
///
/// See explanation on `VtableImplData`.
#[derive(Clone, RustcEncodable, RustcDecodable)]
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable)]
pub enum Vtable<'tcx, N> {
/// Vtable identifying a particular impl.
VtableImpl(VtableImplData<'tcx, N>),
Expand Down Expand Up @@ -374,13 +374,13 @@ pub struct VtableClosureData<'tcx, N> {
pub nested: Vec<N>
}

#[derive(Clone, RustcEncodable, RustcDecodable)]
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable)]
pub struct VtableAutoImplData<N> {
pub trait_def_id: DefId,
pub nested: Vec<N>
}

#[derive(Clone, RustcEncodable, RustcDecodable)]
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable)]
pub struct VtableBuiltinData<N> {
pub nested: Vec<N>
}
Expand Down
175 changes: 92 additions & 83 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use super::translate_substs;
use super::Obligation;
use super::ObligationCause;
use super::PredicateObligation;
use super::Selection;
use super::SelectionContext;
use super::SelectionError;
use super::VtableClosureData;
Expand Down Expand Up @@ -101,7 +102,7 @@ pub struct MismatchedProjectionTypes<'tcx> {
pub err: ty::error::TypeError<'tcx>
}

#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)]
#[derive(PartialEq, Eq, Debug)]
enum ProjectionTyCandidate<'tcx> {
// from a where-clause in the env or object type
ParamEnv(ty::PolyProjectionPredicate<'tcx>),
Expand All @@ -110,7 +111,7 @@ enum ProjectionTyCandidate<'tcx> {
TraitDef(ty::PolyProjectionPredicate<'tcx>),

// from a "impl" (or a "pseudo-impl" returned by select)
Select,
Select(Selection<'tcx>),
}

struct ProjectionTyCandidateSet<'tcx> {
Expand Down Expand Up @@ -818,55 +819,57 @@ fn project_type<'cx, 'gcx, 'tcx>(
&obligation_trait_ref,
&mut candidates);

let decide_commit = |candidates: &mut ProjectionTyCandidateSet<'tcx>| {
debug!("{} candidates, ambiguous={}",
candidates.vec.len(),
candidates.ambiguous);

// Prefer where-clauses. As in select, if there are multiple
// candidates, we prefer where-clause candidates over impls. This
// may seem a bit surprising, since impls are the source of
// "truth" in some sense, but in fact some of the impls that SEEM
// applicable are not, because of nested obligations. Where
// clauses are the safer choice. See the comment on
// `select::SelectionCandidate` and #21974 for more details.
if candidates.vec.len() > 1 {
debug!("retaining param-env candidates only from {:?}", candidates.vec);
candidates.vec.retain(|c| match *c {
ProjectionTyCandidate::ParamEnv(..) => true,
ProjectionTyCandidate::TraitDef(..) |
ProjectionTyCandidate::Select(..) => false,
});
debug!("resulting candidate set: {:?}", candidates.vec);
if candidates.vec.len() != 1 {
candidates.ambiguous = true;
return false;
}
}

assert!(candidates.vec.len() <= 1);
match candidates.vec.get(0) {
Some(&ProjectionTyCandidate::Select(..)) => true,
_ => false,
}
};

// Note that at here, if `ProjectionTyCandidate::Select` is not going to be
// a valid candidate, the closure is not executed at all. There are two cases,
// one being trait selection error, and another being ambiguous candidates.
// We handle both by the early return below.
if let Err(e) = assemble_candidates_from_impls(selcx,
obligation,
&obligation_trait_ref,
&mut candidates) {
&mut candidates,
decide_commit) {
return Err(ProjectionTyError::TraitSelectionError(e));
}

debug!("{} candidates, ambiguous={}",
candidates.vec.len(),
candidates.ambiguous);

// Inherent ambiguity that prevents us from even enumerating the
// candidates.
if candidates.ambiguous {
return Err(ProjectionTyError::TooManyCandidates);
}

// Drop duplicates.
//
// Note: `candidates.vec` seems to be on the critical path of the
// compiler. Replacing it with an HashSet was also tried, which would
// render the following dedup unnecessary. The original comment indicated
// that it was 9% slower, but that data is now obsolete and a new
// benchmark should be performed.
candidates.vec.sort_unstable();
candidates.vec.dedup();

// Prefer where-clauses. As in select, if there are multiple
// candidates, we prefer where-clause candidates over impls. This
// may seem a bit surprising, since impls are the source of
// "truth" in some sense, but in fact some of the impls that SEEM
// applicable are not, because of nested obligations. Where
// clauses are the safer choice. See the comment on
// `select::SelectionCandidate` and #21974 for more details.
if candidates.vec.len() > 1 {
debug!("retaining param-env candidates only from {:?}", candidates.vec);
candidates.vec.retain(|c| match *c {
ProjectionTyCandidate::ParamEnv(..) => true,
ProjectionTyCandidate::TraitDef(..) |
ProjectionTyCandidate::Select => false,
});
debug!("resulting candidate set: {:?}", candidates.vec);
if candidates.vec.len() != 1 {
return Err(ProjectionTyError::TooManyCandidates);
}
}

assert!(candidates.vec.len() <= 1);

match candidates.vec.pop() {
Some(candidate) => {
Ok(ProjectedTy::Progress(
Expand Down Expand Up @@ -962,7 +965,7 @@ fn assemble_candidates_from_predicates<'cx, 'gcx, 'tcx, I>(
debug!("assemble_candidates_from_predicates: predicate={:?}",
predicate);
match predicate {
ty::Predicate::Projection(ref data) => {
ty::Predicate::Projection(data) => {
let same_def_id =
data.0.projection_ty.item_def_id == obligation.predicate.item_def_id;

Expand All @@ -985,26 +988,33 @@ fn assemble_candidates_from_predicates<'cx, 'gcx, 'tcx, I>(
data, is_match, same_def_id);

if is_match {
candidate_set.vec.push(ctor(data.clone()));
candidate_set.vec.push(ctor(data));
}
}
_ => { }
_ => {}
}
}
}

fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
enum NoImplCandidateError<'tcx> {
CanceledCommit,
SelectionError(SelectionError<'tcx>),
}

fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx,
F: FnOnce(&mut ProjectionTyCandidateSet<'tcx>) -> bool>(
selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
obligation: &ProjectionTyObligation<'tcx>,
obligation_trait_ref: &ty::TraitRef<'tcx>,
candidate_set: &mut ProjectionTyCandidateSet<'tcx>)
candidate_set: &mut ProjectionTyCandidateSet<'tcx>,
decide_commit: F)
-> Result<(), SelectionError<'tcx>>
{
// If we are resolving `<T as TraitRef<...>>::Item == Type`,
// start out by selecting the predicate `T as TraitRef<...>`:
let poly_trait_ref = obligation_trait_ref.to_poly_trait_ref();
let trait_obligation = obligation.with(poly_trait_ref.to_poly_trait_predicate());
selcx.infcx().probe(|_| {
match selcx.infcx().commit_if_ok(|_| {
let vtable = match selcx.select(&trait_obligation) {
Ok(Some(vtable)) => vtable,
Ok(None) => {
Expand All @@ -1014,21 +1024,20 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
Err(e) => {
debug!("assemble_candidates_from_impls: selection error {:?}",
e);
return Err(e);
return Err(NoImplCandidateError::SelectionError(e));
}
};

match vtable {
super::VtableClosure(_) |
super::VtableGenerator(_) |
super::VtableFnPointer(_) |
super::VtableObject(_) => {
let eligible = match &vtable {
&super::VtableClosure(_) |
&super::VtableGenerator(_) |
&super::VtableFnPointer(_) |
&super::VtableObject(_) => {
debug!("assemble_candidates_from_impls: vtable={:?}",
vtable);

candidate_set.vec.push(ProjectionTyCandidate::Select);
true
}
super::VtableImpl(ref impl_data) => {
&super::VtableImpl(ref impl_data) => {
// We have to be careful when projecting out of an
// impl because of specialization. If we are not in
// trans (i.e., projection mode is not "any"), and the
Expand Down Expand Up @@ -1072,29 +1081,27 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
node_item.item.defaultness.has_value()
} else {
node_item.item.defaultness.is_default() ||
selcx.tcx().impl_is_default(node_item.node.def_id())
selcx.tcx().impl_is_default(node_item.node.def_id())
};

// Only reveal a specializable default if we're past type-checking
// and the obligations is monomorphic, otherwise passes such as
// transmute checking and polymorphic MIR optimizations could
// get a result which isn't correct for all monomorphizations.
let new_candidate = if !is_default {
Some(ProjectionTyCandidate::Select)
if !is_default {
true
} else if obligation.param_env.reveal == Reveal::All {
assert!(!poly_trait_ref.needs_infer());
if !poly_trait_ref.needs_subst() {
Some(ProjectionTyCandidate::Select)
true
} else {
None
false
}
} else {
None
};

candidate_set.vec.extend(new_candidate);
false
}
}
super::VtableParam(..) => {
&super::VtableParam(..) => {
// This case tell us nothing about the value of an
// associated type. Consider:
//
Expand All @@ -1120,19 +1127,32 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
// in the compiler: a trait predicate (`T : SomeTrait`) and a
// projection. And the projection where clause is handled
// in `assemble_candidates_from_param_env`.
false
}
super::VtableAutoImpl(..) |
super::VtableBuiltin(..) => {
&super::VtableAutoImpl(..) |
&super::VtableBuiltin(..) => {
// These traits have no associated types.
span_bug!(
obligation.cause.span,
"Cannot project an associated type from `{:?}`",
vtable);
}
};

if eligible {
candidate_set.vec.push(ProjectionTyCandidate::Select(vtable));
}

Ok(())
})
if decide_commit(candidate_set) {
Ok(())
} else {
Err(NoImplCandidateError::CanceledCommit)
}
}) {
Ok(()) => Ok(()),
Err(NoImplCandidateError::CanceledCommit) => Ok(()),
Err(NoImplCandidateError::SelectionError(e)) => Err(e),
}
}

fn confirm_candidate<'cx, 'gcx, 'tcx>(
Expand All @@ -1152,30 +1172,19 @@ fn confirm_candidate<'cx, 'gcx, 'tcx>(
confirm_param_env_candidate(selcx, obligation, poly_projection)
}

ProjectionTyCandidate::Select => {
confirm_select_candidate(selcx, obligation, obligation_trait_ref)
ProjectionTyCandidate::Select(vtable) => {
confirm_select_candidate(selcx, obligation, obligation_trait_ref, vtable)
}
}
}

fn confirm_select_candidate<'cx, 'gcx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
obligation: &ProjectionTyObligation<'tcx>,
obligation_trait_ref: &ty::TraitRef<'tcx>)
obligation_trait_ref: &ty::TraitRef<'tcx>,
vtable: Selection<'tcx>)
-> Progress<'tcx>
{
let poly_trait_ref = obligation_trait_ref.to_poly_trait_ref();
let trait_obligation = obligation.with(poly_trait_ref.to_poly_trait_predicate());
let vtable = match selcx.select(&trait_obligation) {
Ok(Some(vtable)) => vtable,
_ => {
span_bug!(
obligation.cause.span,
"Failed to select `{:?}`",
trait_obligation);
}
};

match vtable {
super::VtableImpl(data) =>
confirm_impl_candidate(selcx, obligation, data),
Expand Down
13 changes: 12 additions & 1 deletion src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3282,7 +3282,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// that order.
let predicates = tcx.predicates_of(def_id);
assert_eq!(predicates.parent, None);
let predicates = predicates.predicates.iter().flat_map(|predicate| {
let mut predicates: Vec<_> = predicates.predicates.iter().flat_map(|predicate| {
let predicate = normalize_with_depth(self, param_env, cause.clone(), recursion_depth,
&predicate.subst(tcx, substs));
predicate.obligations.into_iter().chain(
Expand All @@ -3293,6 +3293,17 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
predicate: predicate.value
}))
}).collect();
if predicates.len() > 1 {
let mut i = 0;
while i != predicates.len() {
let has_dup = (0..i).any(|j| predicates[i] == predicates[j]);
if has_dup {
predicates.swap_remove(i);
} else {
i += 1;
}
}
}
self.infcx().plug_leaks(skol_map, snapshot, predicates)
}
}
Expand Down
Loading

0 comments on commit 4ac2425

Please sign in to comment.