Skip to content

Commit

Permalink
fix logic error in #44269's prune_cache_value_obligations
Browse files Browse the repository at this point in the history
We want to retain obligations that *contain* inference variables, not
obligations that *don't contain* them, in order to fix #43132. Because
of surrounding changes to inference, the ICE doesn't occur in its
original case, but I believe it could still be made to occur on master.

Maybe I should try to write a new test case? Certainly not right now
(I'm mainly trying to get us a beta that we can ship) but maybe before
we land this PR on nightly?

This seems to cause a 10% performance regression in my imprecise
attempt to benchmark item-body checking for #43613, but it's better to
be slow and right than fast and wrong. If we want to recover that, I
think we can change the constrained-type-parameter code to actually
give a list of projections that are important for resolving inference
variables and filter everything else out.
  • Loading branch information
arielb1 committed Oct 6, 2017
1 parent 9ae6ed7 commit 91fdadb
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/librustc/traits/project.rs
Expand Up @@ -639,7 +639,7 @@ fn prune_cache_value_obligations<'a, 'gcx, 'tcx>(infcx: &'a InferCtxt<'a, 'gcx,
// but we have `T: Foo<X = ?1>` and `?1: Bar<X =
// ?0>`).
ty::Predicate::Projection(ref data) =>
!infcx.any_unresolved_type_vars(&data.ty()),
infcx.any_unresolved_type_vars(&data.ty()),

// We are only interested in `T: Foo<X = U>` predicates, whre
// `U` references one of `unresolved_type_vars`. =)
Expand Down

0 comments on commit 91fdadb

Please sign in to comment.