Skip to content

Commit

Permalink
use the evaluation cache instead of the global fulfillment cache
Browse files Browse the repository at this point in the history
The evaluation cache already exists, and it can handle differing
parameter environments natively.

Fixes #39970.
Fixes #42796.
  • Loading branch information
arielb1 authored and nikomatsakis committed Jul 7, 2017
1 parent 16d1700 commit 87a1181
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 106 deletions.
95 changes: 18 additions & 77 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use dep_graph::DepGraph;
use infer::{InferCtxt, InferOk};
use ty::{self, Ty, TypeFoldable, ToPolyTraitRef, TyCtxt, ToPredicate};
use ty::{self, Ty, TypeFoldable, ToPolyTraitRef, ToPredicate};
use ty::error::ExpectedFound;
use rustc_data_structures::obligation_forest::{ObligationForest, Error};
use rustc_data_structures::obligation_forest::{ForestObligation, ObligationProcessor};
use std::marker::PhantomData;
use syntax::ast;
use util::nodemap::{FxHashSet, NodeMap};
use util::nodemap::NodeMap;
use hir::def_id::DefId;

use super::CodeAmbiguity;
Expand All @@ -34,11 +33,6 @@ impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> {
fn as_predicate(&self) -> &Self::Predicate { &self.obligation.predicate }
}

pub struct GlobalFulfilledPredicates<'tcx> {
set: FxHashSet<ty::PolyTraitPredicate<'tcx>>,
dep_graph: DepGraph,
}

/// The fulfillment context is used to drive trait resolution. It
/// consists of a list of obligations that must be (eventually)
/// satisfied. The job is to track which are satisfied, which yielded
Expand Down Expand Up @@ -183,13 +177,6 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {

assert!(!infcx.is_in_snapshot());

let tcx = infcx.tcx;

if tcx.fulfilled_predicates.borrow().check_duplicate(tcx, &obligation.predicate) {
debug!("register_predicate_obligation: duplicate");
return
}

self.predicates.register_obligation(PendingPredicateObligation {
obligation,
stalled_on: vec![]
Expand Down Expand Up @@ -264,13 +251,6 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
});
debug!("select: outcome={:?}", outcome);

// these are obligations that were proven to be true.
for pending_obligation in outcome.completed {
let predicate = &pending_obligation.obligation.predicate;
selcx.tcx().fulfilled_predicates.borrow_mut()
.add_if_global(selcx.tcx(), predicate);
}

errors.extend(
outcome.errors.into_iter()
.map(|e| to_fulfillment_error(e)));
Expand Down Expand Up @@ -375,21 +355,31 @@ fn process_predicate<'a, 'gcx, 'tcx>(

match obligation.predicate {
ty::Predicate::Trait(ref data) => {
let tcx = selcx.tcx();
if tcx.fulfilled_predicates.borrow().check_duplicate_trait(tcx, data) {
return Ok(Some(vec![]));
let trait_obligation = obligation.with(data.clone());

if data.is_global() {
// no type variables present, can use evaluation for better caching.
// FIXME: consider caching errors too.
if
// make defaulted unit go through the slow path for better warnings,
// please remove this when the warnings are removed.
!trait_obligation.predicate.skip_binder().self_ty().is_defaulted_unit() &&
selcx.evaluate_obligation_conservatively(&obligation) {
debug!("selecting trait `{:?}` at depth {} evaluated to holds",
data, obligation.recursion_depth);
return Ok(Some(vec![]))
}
}

let trait_obligation = obligation.with(data.clone());
match selcx.select(&trait_obligation) {
Ok(Some(vtable)) => {
debug!("selecting trait `{:?}` at depth {} yielded Ok(Some)",
data, obligation.recursion_depth);
data, obligation.recursion_depth);
Ok(Some(vtable.nested_obligations()))
}
Ok(None) => {
debug!("selecting trait `{:?}` at depth {} yielded Ok(None)",
data, obligation.recursion_depth);
data, obligation.recursion_depth);

// This is a bit subtle: for the most part, the
// only reason we can fail to make progress on
Expand Down Expand Up @@ -568,55 +558,6 @@ fn register_region_obligation<'tcx>(t_a: Ty<'tcx>,

}

impl<'a, 'gcx, 'tcx> GlobalFulfilledPredicates<'gcx> {
pub fn new(dep_graph: DepGraph) -> GlobalFulfilledPredicates<'gcx> {
GlobalFulfilledPredicates {
set: FxHashSet(),
dep_graph,
}
}

pub fn check_duplicate(&self, tcx: TyCtxt, key: &ty::Predicate<'tcx>) -> bool {
if let ty::Predicate::Trait(ref data) = *key {
self.check_duplicate_trait(tcx, data)
} else {
false
}
}

pub fn check_duplicate_trait(&self, tcx: TyCtxt, data: &ty::PolyTraitPredicate<'tcx>) -> bool {
// For the global predicate registry, when we find a match, it
// may have been computed by some other task, so we want to
// add a read from the node corresponding to the predicate
// processing to make sure we get the transitive dependencies.
if self.set.contains(data) {
debug_assert!(data.is_global());
self.dep_graph.read(data.dep_node(tcx));
debug!("check_duplicate: global predicate `{:?}` already proved elsewhere", data);

true
} else {
false
}
}

fn add_if_global(&mut self, tcx: TyCtxt<'a, 'gcx, 'tcx>, key: &ty::Predicate<'tcx>) {
if let ty::Predicate::Trait(ref data) = *key {
// We only add things to the global predicate registry
// after the current task has proved them, and hence
// already has the required read edges, so we don't need
// to add any more edges here.
if data.is_global() {
if let Some(data) = tcx.lift_to_global(data) {
if self.set.insert(data.clone()) {
debug!("add_if_global: global predicate `{:?}` added", data);
}
}
}
}
}
}

fn to_fulfillment_error<'tcx>(
error: Error<PendingPredicateObligation<'tcx>, FulfillmentErrorCode<'tcx>>)
-> FulfillmentError<'tcx>
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use syntax_pos::{Span, DUMMY_SP};
pub use self::coherence::orphan_check;
pub use self::coherence::overlapping_impls;
pub use self::coherence::OrphanCheckErr;
pub use self::fulfill::{FulfillmentContext, GlobalFulfilledPredicates, RegionObligation};
pub use self::fulfill::{FulfillmentContext, RegionObligation};
pub use self::project::MismatchedProjectionTypes;
pub use self::project::{normalize, normalize_projection_type, Normalized};
pub use self::project::{ProjectionCache, ProjectionCacheSnapshot, Reveal};
Expand Down
40 changes: 20 additions & 20 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,21 +514,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("evaluate_predicate_recursively({:?})",
obligation);

let tcx = self.tcx();

// Check the cache from the tcx of predicates that we know
// have been proven elsewhere. This cache only contains
// predicates that are global in scope and hence unaffected by
// the current environment.
if tcx.fulfilled_predicates.borrow().check_duplicate(tcx, &obligation.predicate) {
return EvaluatedToOk;
}

match obligation.predicate {
ty::Predicate::Trait(ref t) => {
assert!(!t.has_escaping_regions());
let obligation = obligation.with(t.clone());
self.evaluate_obligation_recursively(previous_stack, &obligation)
self.evaluate_trait_predicate_recursively(previous_stack, obligation)
}

ty::Predicate::Equate(ref p) => {
Expand Down Expand Up @@ -613,15 +603,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
}
}

fn evaluate_obligation_recursively<'o>(&mut self,
previous_stack: TraitObligationStackList<'o, 'tcx>,
obligation: &TraitObligation<'tcx>)
-> EvaluationResult
fn evaluate_trait_predicate_recursively<'o>(&mut self,
previous_stack: TraitObligationStackList<'o, 'tcx>,
mut obligation: TraitObligation<'tcx>)
-> EvaluationResult
{
debug!("evaluate_obligation_recursively({:?})",
debug!("evaluate_trait_predicate_recursively({:?})",
obligation);

let stack = self.push_stack(previous_stack, obligation);
if !self.intercrate && obligation.is_global() {
// If a param env is consistent, global obligations do not depend on its particular
// value in order to work, so we can clear out the param env and get better
// caching. (If the current param env is inconsistent, we don't care what happens).
debug!("evaluate_trait_predicate_recursively({:?}) - in global", obligation);
obligation.param_env = ty::ParamEnv::empty(obligation.param_env.reveal);
}

let stack = self.push_stack(previous_stack, &obligation);
let fresh_trait_ref = stack.fresh_trait_ref;
if let Some(result) = self.check_evaluation_cache(obligation.param_env, fresh_trait_ref) {
debug!("CACHE HIT: EVAL({:?})={:?}",
Expand Down Expand Up @@ -676,8 +674,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
}
if unbound_input_types &&
stack.iter().skip(1).any(
|prev| self.match_fresh_trait_refs(&stack.fresh_trait_ref,
&prev.fresh_trait_ref))
|prev| stack.obligation.param_env == prev.obligation.param_env &&
self.match_fresh_trait_refs(&stack.fresh_trait_ref,
&prev.fresh_trait_ref))
{
debug!("evaluate_stack({:?}) --> unbound argument, recursive --> giving up",
stack.fresh_trait_ref);
Expand Down Expand Up @@ -706,7 +705,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
if let Some(rec_index) =
stack.iter()
.skip(1) // skip top-most frame
.position(|prev| stack.fresh_trait_ref == prev.fresh_trait_ref)
.position(|prev| stack.obligation.param_env == prev.obligation.param_env &&
stack.fresh_trait_ref == prev.fresh_trait_ref)
{
debug!("evaluate_stack({:?}) --> recursive",
stack.fresh_trait_ref);
Expand Down
8 changes: 0 additions & 8 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,12 +507,6 @@ pub struct GlobalCtxt<'tcx> {
/// Merge this with `selection_cache`?
pub evaluation_cache: traits::EvaluationCache<'tcx>,

/// A set of predicates that have been fulfilled *somewhere*.
/// This is used to avoid duplicate work. Predicates are only
/// added to this set when they mention only "global" names
/// (i.e., no type or lifetime parameters).
pub fulfilled_predicates: RefCell<traits::GlobalFulfilledPredicates<'tcx>>,

/// Maps Expr NodeId's to `true` iff `&expr` can have 'static lifetime.
pub rvalue_promotable_to_static: RefCell<NodeMap<bool>>,

Expand Down Expand Up @@ -686,7 +680,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
let interners = CtxtInterners::new(arena);
let common_types = CommonTypes::new(&interners);
let dep_graph = hir.dep_graph.clone();
let fulfilled_predicates = traits::GlobalFulfilledPredicates::new(dep_graph.clone());
let max_cnum = s.cstore.crates().iter().map(|c| c.as_usize()).max().unwrap_or(0);
let mut providers = IndexVec::from_elem_n(extern_providers, max_cnum + 1);
providers[LOCAL_CRATE] = local_providers;
Expand Down Expand Up @@ -735,7 +728,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
named_region_map,
trait_map: resolutions.trait_map,
export_map: resolutions.export_map,
fulfilled_predicates: RefCell::new(fulfilled_predicates),
hir,
def_path_hash_to_def_id,
maps: maps::Maps::new(providers),
Expand Down
31 changes: 31 additions & 0 deletions src/test/compile-fail/issue-39970.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

trait Array<'a> {
type Element: 'a;
}

trait Visit {
fn visit() {}
}

impl<'a> Array<'a> for () {
type Element = &'a ();
}

impl Visit for () where
//(): for<'a> Array<'a, Element=&'a ()>, // No ICE
(): for<'a> Array<'a, Element=()>, // ICE
{}

fn main() {
<() as Visit>::visit();
//~^ ERROR type mismatch resolving `for<'a> <() as Array<'a>>::Element == ()`
}
29 changes: 29 additions & 0 deletions src/test/compile-fail/issue-42796.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub trait Mirror<Smoke> {
type Image;
}

impl<T, Smoke> Mirror<Smoke> for T {
type Image = T;
}

pub fn poison<S>(victim: String) where <String as Mirror<S>>::Image: Copy {
loop { drop(victim); } //~ ERROR use of moved value
}

fn main() {
let s = "Hello!".to_owned();
let mut s_copy = s;
s_copy.push_str("World!");
"0wned!".to_owned();
println!("{}", s); //~ ERROR use of moved value
}

0 comments on commit 87a1181

Please sign in to comment.