From 1da73ff12642182c9049630c3625f44f718c6c2f Mon Sep 17 00:00:00 2001
From: Niko Matsakis <niko@alum.mit.edu>
Date: Tue, 28 Oct 2014 07:13:15 -0400
Subject: [PATCH] Avoid O(n^2) performance by reconsidering the full set of
obligations only when we are about to report an error (#18208). I found it is
still important to consider the full set in order to make tests like `let x:
Vec<_> = obligations.iter().collect()` work.
---
src/librustc/middle/traits/fulfill.rs | 96 +++++++++++++++++-----
src/librustc/middle/typeck/check/method.rs | 4 +-
src/librustc/middle/typeck/check/mod.rs | 51 ++++++++----
src/librustc/middle/typeck/check/vtable.rs | 18 ++++
4 files changed, 128 insertions(+), 41 deletions(-)
diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs
index c0caa1d7c79..6baf4e6c048 100644
--- a/src/librustc/middle/traits/fulfill.rs
+++ b/src/librustc/middle/traits/fulfill.rs
@@ -35,12 +35,18 @@ pub struct FulfillmentContext {
// A list of all obligations that have been registered with this
// fulfillment context.
trait_obligations: Vec<Obligation>,
+
+ // Remembers the count of trait obligations that we have already
+ // attempted to select. This is used to avoid repeating work
+ // when `select_new_obligations` is called.
+ attempted_mark: uint,
}
impl FulfillmentContext {
pub fn new() -> FulfillmentContext {
FulfillmentContext {
trait_obligations: Vec::new(),
+ attempted_mark: 0,
}
}
@@ -74,18 +80,49 @@ impl FulfillmentContext {
}
}
+ pub fn select_new_obligations<'a,'tcx>(&mut self,
+ infcx: &InferCtxt<'a,'tcx>,
+ param_env: &ty::ParameterEnvironment,
+ typer: &Typer<'tcx>)
+ -> Result<(),Vec<FulfillmentError>>
+ {
+ /*!
+ * Attempts to select obligations that were registered since
+ * the call to a selection routine. This is used by the type checker
+ * to eagerly attempt to resolve obligations in hopes of gaining
+ * type information. It'd be equally valid to use `select_where_possible`
+ * but it results in `O(n^2)` performance (#18208).
+ */
+
+ let mut selcx = SelectionContext::new(infcx, param_env, typer);
+ self.select(&mut selcx, true)
+ }
+
pub fn select_where_possible<'a,'tcx>(&mut self,
infcx: &InferCtxt<'a,'tcx>,
param_env: &ty::ParameterEnvironment,
typer: &Typer<'tcx>)
-> Result<(),Vec<FulfillmentError>>
{
- let tcx = infcx.tcx;
let mut selcx = SelectionContext::new(infcx, param_env, typer);
+ self.select(&mut selcx, false)
+ }
- debug!("select_where_possible({} obligations) start",
- self.trait_obligations.len());
+ fn select(&mut self,
+ selcx: &mut SelectionContext,
+ only_new_obligations: bool)
+ -> Result<(),Vec<FulfillmentError>>
+ {
+ /*!
+ * Attempts to select obligations using `selcx`. If
+ * `only_new_obligations` is true, then it only attempts to
+ * select obligations that haven't been seen before.
+ */
+ debug!("select({} obligations, only_new_obligations={}) start",
+ self.trait_obligations.len(),
+ only_new_obligations);
+ let tcx = selcx.tcx();
let mut errors = Vec::new();
loop {
@@ -96,30 +133,47 @@ impl FulfillmentContext {
let mut selections = Vec::new();
+ // If we are only attempting obligations we haven't seen yet,
+ // then set `skip` to the number of obligations we've already
+ // seen.
+ let mut skip = if only_new_obligations {
+ self.attempted_mark
+ } else {
+ 0
+ };
+
// First pass: walk each obligation, retaining
// only those that we cannot yet process.
self.trait_obligations.retain(|obligation| {
- match selcx.select(obligation) {
- Ok(None) => {
- true
- }
- Ok(Some(s)) => {
- selections.push(s);
- false
- }
- Err(selection_err) => {
- debug!("obligation: {} error: {}",
- obligation.repr(tcx),
- selection_err.repr(tcx));
-
- errors.push(FulfillmentError::new(
- (*obligation).clone(),
- CodeSelectionError(selection_err)));
- false
+ // Hack: Retain does not pass in the index, but we want
+ // to avoid processing the first `start_count` entries.
+ if skip > 0 {
+ skip -= 1;
+ true
+ } else {
+ match selcx.select(obligation) {
+ Ok(None) => {
+ true
+ }
+ Ok(Some(s)) => {
+ selections.push(s);
+ false
+ }
+ Err(selection_err) => {
+ debug!("obligation: {} error: {}",
+ obligation.repr(tcx),
+ selection_err.repr(tcx));
+ errors.push(FulfillmentError::new(
+ (*obligation).clone(),
+ CodeSelectionError(selection_err)));
+ false
+ }
}
}
});
+ self.attempted_mark = self.trait_obligations.len();
+
if self.trait_obligations.len() == count {
// Nothing changed.
break;
@@ -133,7 +187,7 @@ impl FulfillmentContext {
}
}
- debug!("select_where_possible({} obligations, {} errors) done",
+ debug!("select({} obligations, {} errors) done",
self.trait_obligations.len(),
errors.len());
diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs
index f8604eeb5c6..82a3d1a523d 100644
--- a/src/librustc/middle/typeck/check/method.rs
+++ b/src/librustc/middle/typeck/check/method.rs
@@ -88,7 +88,7 @@ use middle::ty;
use middle::typeck::astconv::AstConv;
use middle::typeck::check::{FnCtxt, NoPreference, PreferMutLvalue};
use middle::typeck::check::{impl_self_ty};
-use middle::typeck::check::vtable::select_fcx_obligations_where_possible;
+use middle::typeck::check::vtable::select_new_fcx_obligations;
use middle::typeck::check;
use middle::typeck::infer;
use middle::typeck::{MethodCall, MethodCallee};
@@ -1302,7 +1302,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
// the `Self` trait).
let callee = self.confirm_candidate(rcvr_ty, &candidate);
- select_fcx_obligations_where_possible(self.fcx);
+ select_new_fcx_obligations(self.fcx);
Some(Ok(callee))
}
diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs
index 5f6795f24c1..53a4a8141e6 100644
--- a/src/librustc/middle/typeck/check/mod.rs
+++ b/src/librustc/middle/typeck/check/mod.rs
@@ -101,7 +101,6 @@ use middle::typeck::check::method::{AutoderefReceiver};
use middle::typeck::check::method::{CheckTraitsAndInherentMethods};
use middle::typeck::check::regionmanip::replace_late_bound_regions;
use middle::typeck::CrateCtxt;
-use middle::typeck::infer::{resolve_type, force_tvar};
use middle::typeck::infer;
use middle::typeck::rscope::RegionScope;
use middle::typeck::{lookup_def_ccx};
@@ -1412,7 +1411,7 @@ fn check_cast(fcx: &FnCtxt,
}
// casts from C-like enums are allowed
} else if t_1_is_char {
- let t_e = fcx.infcx().resolve_type_vars_if_possible(t_e);
+ let t_e = fcx.infcx().shallow_resolve(t_e);
if ty::get(t_e).sty != ty::ty_uint(ast::TyU8) {
fcx.type_error_message(span, |actual| {
format!("only `u8` can be cast as \
@@ -2564,7 +2563,7 @@ fn check_argument_types<'a>(fcx: &FnCtxt,
// an "opportunistic" vtable resolution of any trait
// bounds on the call.
if check_blocks {
- vtable::select_fcx_obligations_where_possible(fcx);
+ vtable::select_new_fcx_obligations(fcx);
}
// For variadic functions, we don't have a declared type for all of
@@ -2988,9 +2987,11 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
// 'else' branch.
let expected = match expected.only_has_type() {
ExpectHasType(ety) => {
- match infer::resolve_type(fcx.infcx(), Some(sp), ety, force_tvar) {
- Ok(rty) if !ty::type_is_ty_var(rty) => ExpectHasType(rty),
- _ => NoExpectation
+ let ety = fcx.infcx().shallow_resolve(ety);
+ if !ty::type_is_ty_var(ety) {
+ ExpectHasType(ety)
+ } else {
+ NoExpectation
}
}
_ => NoExpectation
@@ -4037,7 +4038,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
ast::ExprForLoop(ref pat, ref head, ref block, _) => {
check_expr(fcx, &**head);
let typ = lookup_method_for_for_loop(fcx, &**head, expr.id);
- vtable::select_fcx_obligations_where_possible(fcx);
+ vtable::select_new_fcx_obligations(fcx);
let pcx = pat_ctxt {
fcx: fcx,
@@ -5393,18 +5394,32 @@ pub fn instantiate_path(fcx: &FnCtxt,
// Resolves `typ` by a single level if `typ` is a type variable. If no
// resolution is possible, then an error is reported.
-pub fn structurally_resolved_type(fcx: &FnCtxt, sp: Span, tp: ty::t) -> ty::t {
- match infer::resolve_type(fcx.infcx(), Some(sp), tp, force_tvar) {
- Ok(t_s) if !ty::type_is_ty_var(t_s) => t_s,
- _ => {
- fcx.type_error_message(sp, |_actual| {
- "the type of this value must be known in this \
- context".to_string()
- }, tp, None);
- demand::suptype(fcx, sp, ty::mk_err(), tp);
- tp
- }
+pub fn structurally_resolved_type(fcx: &FnCtxt, sp: Span, mut ty: ty::t) -> ty::t {
+ // If `ty` is a type variable, see whether we already know what it is.
+ ty = fcx.infcx().shallow_resolve(ty);
+
+ // If not, try resolve pending fcx obligations. Those can shed light.
+ //
+ // FIXME(#18391) -- This current strategy can lead to bad performance in
+ // extreme cases. We probably ought to smarter in general about
+ // only resolving when we need help and only resolving obligations
+ // will actually help.
+ if ty::type_is_ty_var(ty) {
+ vtable::select_fcx_obligations_where_possible(fcx);
+ ty = fcx.infcx().shallow_resolve(ty);
}
+
+ // If not, error.
+ if ty::type_is_ty_var(ty) {
+ fcx.type_error_message(sp, |_actual| {
+ "the type of this value must be known in this \
+ context".to_string()
+ }, ty, None);
+ demand::suptype(fcx, sp, ty::mk_err(), ty);
+ ty = ty::mk_err();
+ }
+
+ ty
}
// Returns the one-level-deep structure of the given type.
diff --git a/src/librustc/middle/typeck/check/vtable.rs b/src/librustc/middle/typeck/check/vtable.rs
index d557a2b713b..08a1e95fd90 100644
--- a/src/librustc/middle/typeck/check/vtable.rs
+++ b/src/librustc/middle/typeck/check/vtable.rs
@@ -339,6 +339,24 @@ pub fn select_fcx_obligations_where_possible(fcx: &FnCtxt) {
}
}
+pub fn select_new_fcx_obligations(fcx: &FnCtxt) {
+ /*!
+ * Try to select any fcx obligation that we haven't tried yet,
+ * in an effort to improve inference. You could just call
+ * `select_fcx_obligations_where_possible` except that it leads
+ * to repeated work.
+ */
+
+ match
+ fcx.inh.fulfillment_cx
+ .borrow_mut()
+ .select_new_obligations(fcx.infcx(), &fcx.inh.param_env, fcx)
+ {
+ Ok(()) => { }
+ Err(errors) => { report_fulfillment_errors(fcx, &errors); }
+ }
+}
+
fn note_obligation_cause(fcx: &FnCtxt,
obligation: &Obligation) {
let tcx = fcx.tcx();