Skip to content

Commit

Permalink
Auto merge of #33816 - nikomatsakis:projection-cache-2, r=arielb1
Browse files Browse the repository at this point in the history
Projection cache and better warnings for #32330

This PR does three things:

- it lays the groundwork for the more precise subtyping rules discussed in #32330, but does not enable them;
- it issues warnings when the result of a leak-check or subtyping check relies on a late-bound region which will late become early-bound when #32330 is fixed;
- it introduces a cache for projection in the inference context.

I'm not 100% happy with the approach taken by the cache here, but it seems like a step in the right direction. It results in big wins on some test cases, but not as big as previous versions -- I think because it is caching the `Vec<Obligation>` (whereas before I just returned the normalized type with an empty vector). However, that change was needed to fix an ICE in @alexcrichton's future-rs module (I haven't fully tracked the cause of that ICE yet). Also, because trans/the collector use a fresh inference context for every call to `fulfill_obligation`, they don't profit nearly as much from this cache as they ought to.

Still, here are the results from the future-rs `retry.rs`:

```
06:26 <nmatsakis> time: 6.246; rss: 44MB  item-bodies checking
06:26 <nmatsakis> time: 54.783; rss: 63MB   translation item collection
06:26 <nmatsakis> time: 140.086; rss: 86MB    translation

06:26 <nmatsakis> time: 0.361; rss: 46MB  item-bodies checking
06:26 <nmatsakis> time: 5.299; rss: 63MB    translation item collection
06:26 <nmatsakis> time: 12.140; rss: 86MB translation
```

~~Another example is the example from #31849. For that, I get 34s to run item-bodies without any cache. The version of the cache included here takes 2s to run item-bodies type-checking. An alternative version which doesn't track nested obligations takes 0.2s, but that version ICEs on @alexcrichton's future-rs (and may well be incorrect, I've not fully convinced myself of that). So, a definite win, but I think there's definitely room for further progress.~~

Pushed a modified version which improves performance of the case from #31849:

```
lunch-box. time rustc --stage0 ~/tmp/issue-31849.rs  -Z no-trans
real    0m33.539s
user    0m32.932s
sys     0m0.570s
lunch-box. time rustc --stage2 ~/tmp/issue-31849.rs  -Z no-trans
real    0m0.195s
user    0m0.154s
sys     0m0.042s
```

Some sort of cache is also needed for unblocking further work on lazy normalization, since that will lean even more heavily on the cache, and will also require cycle detection.

r? @arielb1
  • Loading branch information
bors committed Jun 4, 2016
2 parents 382ab92 + 480d18c commit 12238b9
Show file tree
Hide file tree
Showing 49 changed files with 2,149 additions and 633 deletions.
2 changes: 1 addition & 1 deletion src/librustc/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1647,5 +1647,5 @@ register_diagnostics! {
E0490, // a value of type `..` is borrowed for too long
E0491, // in type `..`, reference has a longer lifetime than the data it...
E0495, // cannot infer an appropriate lifetime due to conflicting requirements
E0525, // expected a closure that implements `..` but this closure only implements `..`
E0525 // expected a closure that implements `..` but this closure only implements `..`
}
54 changes: 31 additions & 23 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ pub trait Visitor<'v> : Sized {
fn visit_generics(&mut self, g: &'v Generics) {
walk_generics(self, g)
}
fn visit_where_predicate(&mut self, predicate: &'v WherePredicate) {
walk_where_predicate(self, predicate)
}
fn visit_fn(&mut self, fk: FnKind<'v>, fd: &'v FnDecl, b: &'v Block, s: Span, _: NodeId) {
walk_fn(self, fk, fd, b, s)
}
Expand Down Expand Up @@ -529,29 +532,34 @@ pub fn walk_generics<'v, V: Visitor<'v>>(visitor: &mut V, generics: &'v Generics
walk_list!(visitor, visit_ty, &param.default);
}
walk_list!(visitor, visit_lifetime_def, &generics.lifetimes);
for predicate in &generics.where_clause.predicates {
match predicate {
&WherePredicate::BoundPredicate(WhereBoundPredicate{ref bounded_ty,
ref bounds,
ref bound_lifetimes,
..}) => {
visitor.visit_ty(bounded_ty);
walk_list!(visitor, visit_ty_param_bound, bounds);
walk_list!(visitor, visit_lifetime_def, bound_lifetimes);
}
&WherePredicate::RegionPredicate(WhereRegionPredicate{ref lifetime,
ref bounds,
..}) => {
visitor.visit_lifetime(lifetime);
walk_list!(visitor, visit_lifetime, bounds);
}
&WherePredicate::EqPredicate(WhereEqPredicate{id,
ref path,
ref ty,
..}) => {
visitor.visit_path(path, id);
visitor.visit_ty(ty);
}
walk_list!(visitor, visit_where_predicate, &generics.where_clause.predicates);
}

pub fn walk_where_predicate<'v, V: Visitor<'v>>(
visitor: &mut V,
predicate: &'v WherePredicate)
{
match predicate {
&WherePredicate::BoundPredicate(WhereBoundPredicate{ref bounded_ty,
ref bounds,
ref bound_lifetimes,
..}) => {
visitor.visit_ty(bounded_ty);
walk_list!(visitor, visit_ty_param_bound, bounds);
walk_list!(visitor, visit_lifetime_def, bound_lifetimes);
}
&WherePredicate::RegionPredicate(WhereRegionPredicate{ref lifetime,
ref bounds,
..}) => {
visitor.visit_lifetime(lifetime);
walk_list!(visitor, visit_lifetime, bounds);
}
&WherePredicate::EqPredicate(WhereEqPredicate{id,
ref path,
ref ty,
..}) => {
visitor.visit_path(path, id);
visitor.visit_ty(ty);
}
}
}
Expand Down
27 changes: 25 additions & 2 deletions src/librustc/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ use hir::map as ast_map;
use hir;
use hir::print as pprust;

use lint;
use hir::def::Def;
use hir::def_id::DefId;
use infer::{self, TypeOrigin};
Expand Down Expand Up @@ -1017,6 +1018,27 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
let (fn_decl, generics) = rebuilder.rebuild();
self.give_expl_lifetime_param(err, &fn_decl, unsafety, constness, name, &generics, span);
}

pub fn issue_32330_warnings(&self, span: Span, issue32330s: &[ty::Issue32330]) {
for issue32330 in issue32330s {
match *issue32330 {
ty::Issue32330::WontChange => { }
ty::Issue32330::WillChange { fn_def_id, region_name } => {
self.tcx.sess.add_lint(
lint::builtin::HR_LIFETIME_IN_ASSOC_TYPE,
ast::CRATE_NODE_ID,
span,
format!("lifetime parameter `{0}` declared on fn `{1}` \
appears only in the return type, \
but here is required to be higher-ranked, \
which means that `{0}` must appear in both \
argument and return types",
region_name,
self.tcx.item_path_str(fn_def_id)));
}
}
}
}
}

struct RebuildPathInfo<'a> {
Expand Down Expand Up @@ -1129,7 +1151,7 @@ impl<'a, 'gcx, 'tcx> Rebuilder<'a, 'gcx, 'tcx> {
ty::BrAnon(i) => {
anon_nums.insert(i);
}
ty::BrNamed(_, name) => {
ty::BrNamed(_, name, _) => {
region_names.insert(name);
}
_ => ()
Expand All @@ -1143,7 +1165,7 @@ impl<'a, 'gcx, 'tcx> Rebuilder<'a, 'gcx, 'tcx> {
for sr in self.same_regions {
for br in &sr.regions {
match *br {
ty::BrNamed(_, name) => {
ty::BrNamed(_, name, _) => {
all_region_names.insert(name);
}
_ => ()
Expand Down Expand Up @@ -1923,3 +1945,4 @@ fn name_to_dummy_lifetime(name: ast::Name) -> hir::Lifetime {
span: codemap::DUMMY_SP,
name: name }
}

Loading

0 comments on commit 12238b9

Please sign in to comment.