From f0f13f86ef434809282bc5eca1bd04a23fdf88d2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 11 May 2015 17:02:56 -0400 Subject: [PATCH] Rather than expanding the where-clauses in the environment over and over again, do it once and then just remember the expanded form. At the same time, filter globally nameable predicates out of the environment, since they can cause cache errors (and they are not necessary in any case). --- src/librustc/middle/traits/mod.rs | 29 ++++++++++---- src/librustc/middle/traits/project.rs | 20 ++++++---- src/librustc/middle/traits/select.rs | 10 ++--- src/librustc/middle/ty.rs | 2 +- src/test/compile-fail/cross-fn-cache-hole.rs | 41 ++++++++++++++++++++ 5 files changed, 79 insertions(+), 23 deletions(-) create mode 100644 src/test/compile-fail/cross-fn-cache-hole.rs diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs index 138231b5995..50536f586e7 100644 --- a/src/librustc/middle/traits/mod.rs +++ b/src/librustc/middle/traits/mod.rs @@ -414,9 +414,27 @@ pub fn normalize_param_env_or_error<'a,'tcx>(unnormalized_env: ty::ParameterEnvi debug!("normalize_param_env_or_error(unnormalized_env={})", unnormalized_env.repr(tcx)); + let predicates: Vec<_> = + util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.clone()) + .filter(|p| !p.is_global()) // (*) + .collect(); + + // (*) Any predicate like `i32: Trait` or whatever doesn't + // need to be in the *environment* to be proven, so screen those + // out. This is important for the soundness of inter-fn + // caching. Note though that we should probably check that these + // predicates hold at the point where the environment is + // constructed, but I am not currently doing so out of laziness. + // -nmatsakis + + debug!("normalize_param_env_or_error: elaborated-predicates={}", + predicates.repr(tcx)); + + let elaborated_env = unnormalized_env.with_caller_bounds(predicates); + let infcx = infer::new_infer_ctxt(tcx); - let predicates = match fully_normalize(&infcx, &unnormalized_env, cause, - &unnormalized_env.caller_bounds) { + let predicates = match fully_normalize(&infcx, &elaborated_env, cause, + &elaborated_env.caller_bounds) { Ok(predicates) => predicates, Err(errors) => { report_fulfillment_errors(&infcx, &errors); @@ -438,14 +456,11 @@ pub fn normalize_param_env_or_error<'a,'tcx>(unnormalized_env: ty::ParameterEnvi // all things considered. let err_msg = fixup_err_to_string(fixup_err); tcx.sess.span_err(span, &err_msg); - return unnormalized_env; // an unnormalized env is better than nothing + return elaborated_env; // an unnormalized env is better than nothing } }; - debug!("normalize_param_env_or_error: predicates={}", - predicates.repr(tcx)); - - unnormalized_env.with_caller_bounds(predicates) + elaborated_env.with_caller_bounds(predicates) } pub fn fully_normalize<'a,'tcx,T>(infcx: &InferCtxt<'a,'tcx>, diff --git a/src/librustc/middle/traits/project.rs b/src/librustc/middle/traits/project.rs index 1631a33588c..6a9ed1a8cf9 100644 --- a/src/librustc/middle/traits/project.rs +++ b/src/librustc/middle/traits/project.rs @@ -531,7 +531,7 @@ fn assemble_candidates_from_param_env<'cx,'tcx>( obligation_trait_ref: &ty::TraitRef<'tcx>, candidate_set: &mut ProjectionTyCandidateSet<'tcx>) { - let env_predicates = selcx.param_env().caller_bounds.clone(); + let env_predicates = selcx.param_env().caller_bounds.iter().cloned(); assemble_candidates_from_predicates(selcx, obligation, obligation_trait_ref, candidate_set, env_predicates); } @@ -567,22 +567,25 @@ fn assemble_candidates_from_trait_def<'cx,'tcx>( // If so, extract what we know from the trait and try to come up with a good answer. let trait_predicates = ty::lookup_predicates(selcx.tcx(), trait_ref.def_id); let bounds = trait_predicates.instantiate(selcx.tcx(), trait_ref.substs); + let bounds = elaborate_predicates(selcx.tcx(), bounds.predicates.into_vec()); assemble_candidates_from_predicates(selcx, obligation, obligation_trait_ref, - candidate_set, bounds.predicates.into_vec()); + candidate_set, bounds) } -fn assemble_candidates_from_predicates<'cx,'tcx>( +fn assemble_candidates_from_predicates<'cx,'tcx,I>( selcx: &mut SelectionContext<'cx,'tcx>, obligation: &ProjectionTyObligation<'tcx>, obligation_trait_ref: &ty::TraitRef<'tcx>, candidate_set: &mut ProjectionTyCandidateSet<'tcx>, - env_predicates: Vec>) + env_predicates: I) + where I: Iterator> { - debug!("assemble_candidates_from_predicates(obligation={}, env_predicates={})", - obligation.repr(selcx.tcx()), - env_predicates.repr(selcx.tcx())); + debug!("assemble_candidates_from_predicates(obligation={})", + obligation.repr(selcx.tcx())); let infcx = selcx.infcx(); - for predicate in elaborate_predicates(selcx.tcx(), env_predicates) { + for predicate in env_predicates { + debug!("assemble_candidates_from_predicates: predicate={}", + predicate.repr(selcx.tcx())); match predicate { ty::Predicate::Projection(ref data) => { let same_name = data.item_name() == obligation.predicate.item_name; @@ -637,6 +640,7 @@ fn assemble_candidates_from_object_type<'cx,'tcx>( let env_predicates = projection_bounds.iter() .map(|p| p.as_predicate()) .collect(); + let env_predicates = elaborate_predicates(selcx.tcx(), env_predicates); assemble_candidates_from_predicates(selcx, obligation, obligation_trait_ref, candidate_set, env_predicates) } diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs index 307242d18df..211839fbd25 100644 --- a/src/librustc/middle/traits/select.rs +++ b/src/librustc/middle/traits/select.rs @@ -1083,14 +1083,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { debug!("assemble_candidates_from_caller_bounds({})", stack.obligation.repr(self.tcx())); - let caller_trait_refs: Vec<_> = - self.param_env().caller_bounds.iter() - .filter_map(|o| o.to_opt_poly_trait_ref()) - .collect(); - let all_bounds = - util::transitive_bounds( - self.tcx(), &caller_trait_refs[..]); + self.param_env().caller_bounds + .iter() + .filter_map(|o| o.to_opt_poly_trait_ref()); let matching_bounds = all_bounds.filter( diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index edd8f5ea6ea..62682e1a6fb 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -2371,7 +2371,7 @@ pub struct ParameterEnvironment<'a, 'tcx:'a> { /// Obligations that the caller must satisfy. This is basically /// the set of bounds on the in-scope type parameters, translated - /// into Obligations. + /// into Obligations, and elaborated and normalized. pub caller_bounds: Vec>, /// Caches the results of trait selection. This cache is used diff --git a/src/test/compile-fail/cross-fn-cache-hole.rs b/src/test/compile-fail/cross-fn-cache-hole.rs new file mode 100644 index 00000000000..0aefd0ae288 --- /dev/null +++ b/src/test/compile-fail/cross-fn-cache-hole.rs @@ -0,0 +1,41 @@ +// Copyright 2012 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Check that when there are vacuous predicates in the environment +// (which make a fn uncallable) we don't erroneously cache those and +// then consider them satisfied elsewhere. The current technique for +// doing this is just to filter "global" predicates out of the +// environment, which means that we wind up with an error in the +// function `vacuous`, because even though `i32: Bar` is implied +// by its where clause, that where clause never holds. + +trait Foo: Bar { +} + +trait Bar { } + +fn vacuous() + where i32: Foo +{ + // vacuous could never be called, because it requires that i32: + // Bar. But the code doesn't check that this could never be + // satisfied. + require::(); + //~^ ERROR the trait `Bar` is not implemented for the type `i32` +} + +fn require() + where A: Bar +{ +} + +fn main() { + require::(); +}