From ff89fcf83b41c2759d69c01d5e41b9043f5acff8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 9 Jun 2015 17:02:18 -0400 Subject: [PATCH 1/4] Add a (somewhat hacky) cache to the tcx that tracks "global" trait refs that are known to have been satisfied *somewhere*. This means that if one fn finds that `SomeType: Foo`, then every other fn can just consider that to hold. Unfortunately, there are some complications: 1. If `SomeType: Foo` includes dependent conditions, those conditions may trigger an error. This error will be repored in the first fn where `SomeType: Foo` is evaluated, but not in the other fns, which can lead to uneven error reporting (which is sometimes confusing). 2. This kind of caching can be unsound in the presence of unsatisfiable where clauses. For example, suppose that the first fn has a where-clause like `i32: Bar`, which in fact does not hold. This will "fool" trait resolution into thinking that `i32: Bar` holds. This is ok currently, because it means that the first fn can never be calle (since its where clauses cannot be satisfied), but if the first fn's successful resolution is cached, it can allow other fns to compile that should not. This problem is fixed in the next commit. --- src/librustc/middle/check_const.rs | 2 +- src/librustc/middle/traits/fulfill.rs | 72 +++++++++++++++++++-- src/librustc/middle/traits/mod.rs | 6 +- src/librustc/middle/traits/select.rs | 8 +++ src/librustc/middle/ty.rs | 57 +++++++++++++++- src/librustc_trans/trans/common.rs | 4 +- src/librustc_trans/trans/monomorphize.rs | 2 +- src/librustc_typeck/check/compare_method.rs | 4 +- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/coherence/mod.rs | 2 +- 10 files changed, 142 insertions(+), 17 deletions(-) diff --git a/src/librustc/middle/check_const.rs b/src/librustc/middle/check_const.rs index 125de300db169..d80eedf354a0e 100644 --- a/src/librustc/middle/check_const.rs +++ b/src/librustc/middle/check_const.rs @@ -285,7 +285,7 @@ impl<'a, 'tcx> CheckCrateVisitor<'a, 'tcx> { fn check_static_type(&self, e: &ast::Expr) { let ty = ty::node_id_to_type(self.tcx, e.id); let infcx = infer::new_infer_ctxt(self.tcx); - let mut fulfill_cx = traits::FulfillmentContext::new(); + let mut fulfill_cx = traits::FulfillmentContext::new(false); let cause = traits::ObligationCause::new(e.span, e.id, traits::SharedStatic); fulfill_cx.register_builtin_bound(&infcx, ty, ty::BoundSync, cause); let env = ty::empty_parameter_environment(self.tcx); diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs index 80acb9bcc1399..b9117745db2e2 100644 --- a/src/librustc/middle/traits/fulfill.rs +++ b/src/librustc/middle/traits/fulfill.rs @@ -28,6 +28,10 @@ use super::select::SelectionContext; use super::Unimplemented; use super::util::predicate_for_builtin_bound; +pub struct FulfilledPredicates<'tcx> { + set: HashSet> +} + /// 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 @@ -44,7 +48,7 @@ pub struct FulfillmentContext<'tcx> { // than the `SelectionCache`: it avoids duplicate errors and // permits recursive obligations, which are often generated from // traits like `Send` et al. - duplicate_set: HashSet>, + duplicate_set: FulfilledPredicates<'tcx>, // A list of all obligations that have been registered with this // fulfillment context. @@ -80,6 +84,8 @@ pub struct FulfillmentContext<'tcx> { // obligations (otherwise, it's easy to fail to walk to a // particular node-id). region_obligations: NodeMap>>, + + errors_will_be_reported: bool, } #[derive(Clone)] @@ -90,12 +96,30 @@ pub struct RegionObligation<'tcx> { } impl<'tcx> FulfillmentContext<'tcx> { - pub fn new() -> FulfillmentContext<'tcx> { + /// Creates a new fulfillment context. + /// + /// `errors_will_be_reported` indicates whether ALL errors that + /// are generated by this fulfillment context will be reported to + /// the end user. This is used to inform caching, because it + /// allows us to conclude that traits that resolve successfully + /// will in fact always resolve successfully (in particular, it + /// guarantees that if some dependent obligation encounters a + /// problem, compilation will be aborted). If you're not sure of + /// the right value here, pass `false`, as that is the more + /// conservative option. + /// + /// FIXME -- a better option would be to hold back on modifying + /// the global cache until we know that all dependent obligations + /// are also satisfied. In that case, we could actually remove + /// this boolean flag, and we'd also avoid the problem of squelching + /// duplicate errors that occur across fns. + pub fn new(errors_will_be_reported: bool) -> FulfillmentContext<'tcx> { FulfillmentContext { - duplicate_set: HashSet::new(), + duplicate_set: FulfilledPredicates::new(), predicates: Vec::new(), attempted_mark: 0, region_obligations: NodeMap(), + errors_will_be_reported: errors_will_be_reported, } } @@ -165,7 +189,7 @@ impl<'tcx> FulfillmentContext<'tcx> { assert!(!obligation.has_escaping_regions()); - if !self.duplicate_set.insert(obligation.predicate.clone()) { + if self.is_duplicate_or_add(infcx.tcx, &obligation.predicate) { debug!("register_predicate({}) -- already seen, skip", obligation.repr(infcx.tcx)); return; } @@ -231,6 +255,28 @@ impl<'tcx> FulfillmentContext<'tcx> { &self.predicates } + fn is_duplicate_or_add(&mut self, tcx: &ty::ctxt<'tcx>, + predicate: &ty::Predicate<'tcx>) + -> bool { + // This is a kind of dirty hack to allow us to avoid "rederiving" + // things that we have already proven in other methods. + // + // The idea is that any predicate that doesn't involve type + // parameters and which only involves the 'static region (and + // no other regions) is universally solvable, since impls are global. + // + // This is particularly important since even if we have a + // cache hit in the selection context, we still wind up + // evaluating the 'nested obligations'. This cache lets us + // skip those. + + if self.errors_will_be_reported && predicate.is_global() { + tcx.fulfilled_predicates.borrow_mut().is_duplicate_or_add(predicate) + } else { + self.duplicate_set.is_duplicate_or_add(predicate) + } + } + /// 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. fn select<'a>(&mut self, @@ -442,3 +488,21 @@ fn register_region_obligation<'tcx>(tcx: &ty::ctxt<'tcx>, .push(region_obligation); } + +impl<'tcx> FulfilledPredicates<'tcx> { + pub fn new() -> FulfilledPredicates<'tcx> { + FulfilledPredicates { + set: HashSet::new() + } + } + + pub fn is_duplicate(&self, p: &ty::Predicate<'tcx>) -> bool { + self.set.contains(p) + } + + fn is_duplicate_or_add(&mut self, p: &ty::Predicate<'tcx>) -> bool { + !self.set.insert(p.clone()) + } +} + + diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs index b371ede0397e2..138231b599586 100644 --- a/src/librustc/middle/traits/mod.rs +++ b/src/librustc/middle/traits/mod.rs @@ -32,7 +32,7 @@ pub use self::error_reporting::suggest_new_overflow_limit; pub use self::coherence::orphan_check; pub use self::coherence::overlapping_impls; pub use self::coherence::OrphanCheckErr; -pub use self::fulfill::{FulfillmentContext, RegionObligation}; +pub use self::fulfill::{FulfillmentContext, FulfilledPredicates, RegionObligation}; pub use self::project::MismatchedProjectionTypes; pub use self::project::normalize; pub use self::project::Normalized; @@ -315,7 +315,7 @@ pub fn evaluate_builtin_bound<'a,'tcx>(infcx: &InferCtxt<'a,'tcx>, ty.repr(infcx.tcx), bound); - let mut fulfill_cx = FulfillmentContext::new(); + let mut fulfill_cx = FulfillmentContext::new(false); // We can use a dummy node-id here because we won't pay any mind // to region obligations that arise (there shouldn't really be any @@ -460,7 +460,7 @@ pub fn fully_normalize<'a,'tcx,T>(infcx: &InferCtxt<'a,'tcx>, debug!("normalize_param_env(value={})", value.repr(tcx)); let mut selcx = &mut SelectionContext::new(infcx, closure_typer); - let mut fulfill_cx = FulfillmentContext::new(); + let mut fulfill_cx = FulfillmentContext::new(false); let Normalized { value: normalized_value, obligations } = project::normalize(selcx, cause, value); debug!("normalize_param_env: normalized_value={} obligations={}", diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs index 5d8cdd44eaa53..307242d18dfef 100644 --- a/src/librustc/middle/traits/select.rs +++ b/src/librustc/middle/traits/select.rs @@ -435,6 +435,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { debug!("evaluate_predicate_recursively({})", obligation.repr(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 self.tcx().fulfilled_predicates.borrow().is_duplicate(&obligation.predicate) { + return EvaluatedToOk; + } + match obligation.predicate { ty::Predicate::Trait(ref t) => { assert!(!t.has_escaping_regions()); diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index c24f1d3e7bedb..edd8f5ea6eaa9 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -753,6 +753,11 @@ pub struct ctxt<'tcx> { /// for things that do not have to do with the parameters in scope. pub selection_cache: traits::SelectionCache<'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 + pub fulfilled_predicates: RefCell>, + /// Caches the representation hints for struct definitions. pub repr_hint_cache: RefCell>>>, @@ -815,6 +820,11 @@ bitflags! { const HAS_TY_ERR = 1 << 6, const HAS_PROJECTION = 1 << 7, const HAS_TY_CLOSURE = 1 << 8, + + // true if there are "names" of types and regions and so forth + // that are local to a particular fn + const HAS_LOCAL_NAMES = 1 << 8, + const NEEDS_SUBST = TypeFlags::HAS_PARAMS.bits | TypeFlags::HAS_SELF.bits | TypeFlags::HAS_RE_EARLY_BOUND.bits, @@ -830,7 +840,8 @@ bitflags! { TypeFlags::HAS_FREE_REGIONS.bits | TypeFlags::HAS_TY_ERR.bits | TypeFlags::HAS_PROJECTION.bits | - TypeFlags::HAS_TY_CLOSURE.bits, + TypeFlags::HAS_TY_CLOSURE.bits | + TypeFlags::HAS_LOCAL_NAMES.bits, // Caches for type_is_sized, type_moves_by_default const SIZEDNESS_CACHED = 1 << 16, @@ -986,6 +997,9 @@ pub fn type_has_ty_infer(ty: Ty) -> bool { pub fn type_needs_infer(ty: Ty) -> bool { ty.flags.get().intersects(TypeFlags::HAS_TY_INFER | TypeFlags::HAS_RE_INFER) } +pub fn type_is_global(ty: Ty) -> bool { + !ty.flags.get().intersects(TypeFlags::HAS_LOCAL_NAMES) +} pub fn type_has_projection(ty: Ty) -> bool { ty.flags.get().intersects(TypeFlags::HAS_PROJECTION) } @@ -1288,6 +1302,15 @@ pub struct UpvarBorrow { pub type UpvarCaptureMap = FnvHashMap; impl Region { + pub fn is_global(&self) -> bool { + // does this represent a region that can be named in a global + // way? used in fulfillment caching. + match *self { + ty::ReStatic | ty::ReEmpty => true, + _ => false, + } + } + pub fn is_bound(&self) -> bool { match *self { ty::ReEarlyBound(..) => true, @@ -2022,6 +2045,29 @@ impl<'tcx> Predicate<'tcx> { Predicate::Projection(ty::Binder(data.subst(tcx, substs))), } } + + // Indicates whether this predicate references only 'global' + // types/lifetimes that are the same regardless of what fn we are + // in. This is used for caching. Errs on the side of returning + // false. + pub fn is_global(&self) -> bool { + match *self { + ty::Predicate::Trait(ref data) => { + let substs = data.skip_binder().trait_ref.substs; + + substs.types.iter().all(|t| ty::type_is_global(t)) && { + match substs.regions { + subst::ErasedRegions => true, + subst::NonerasedRegions(ref r) => r.iter().all(|r| r.is_global()), + } + } + } + + _ => { + false + } + } + } } #[derive(Clone, PartialEq, Eq, Hash, Debug)] @@ -2798,6 +2844,7 @@ pub fn mk_ctxt<'tcx>(s: Session, trait_defs: RefCell::new(DefIdMap()), predicates: RefCell::new(DefIdMap()), super_predicates: RefCell::new(DefIdMap()), + fulfilled_predicates: RefCell::new(traits::FulfilledPredicates::new()), map: map, freevars: freevars, tcache: RefCell::new(DefIdMap()), @@ -3010,6 +3057,7 @@ impl FlagComputation { } &TyParam(ref p) => { + self.add_flags(TypeFlags::HAS_LOCAL_NAMES); if p.space == subst::SelfSpace { self.add_flags(TypeFlags::HAS_SELF); } else { @@ -3018,11 +3066,12 @@ impl FlagComputation { } &TyClosure(_, substs) => { - self.add_flags(TypeFlags::HAS_TY_CLOSURE); + self.add_flags(TypeFlags::HAS_LOCAL_NAMES); self.add_substs(substs); } &TyInfer(_) => { + self.add_flags(TypeFlags::HAS_LOCAL_NAMES); // it might, right? self.add_flags(TypeFlags::HAS_TY_INFER) } @@ -3102,6 +3151,10 @@ impl FlagComputation { ty::ReStatic => {} _ => { self.add_flags(TypeFlags::HAS_FREE_REGIONS); } } + + if !r.is_global() { + self.add_flags(TypeFlags::HAS_LOCAL_NAMES); + } } fn add_projection_predicate(&mut self, projection_predicate: &ProjectionPredicate) { diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs index 0f8fd45b05ac1..1cabeb268562d 100644 --- a/src/librustc_trans/trans/common.rs +++ b/src/librustc_trans/trans/common.rs @@ -1041,7 +1041,7 @@ pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Currently, we use a fulfillment context to completely resolve // all nested obligations. This is because they can inform the // inference of the impl's type parameters. - let mut fulfill_cx = traits::FulfillmentContext::new(); + let mut fulfill_cx = traits::FulfillmentContext::new(true); let vtable = selection.map(|predicate| { fulfill_cx.register_predicate_obligation(&infcx, predicate); }); @@ -1069,7 +1069,7 @@ pub fn normalize_and_test_predicates<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let infcx = infer::new_infer_ctxt(tcx); let typer = NormalizingClosureTyper::new(tcx); let mut selcx = traits::SelectionContext::new(&infcx, &typer); - let mut fulfill_cx = traits::FulfillmentContext::new(); + let mut fulfill_cx = traits::FulfillmentContext::new(false); let cause = traits::ObligationCause::dummy(); let traits::Normalized { value: predicates, obligations } = traits::normalize(&mut selcx, cause.clone(), &predicates); diff --git a/src/librustc_trans/trans/monomorphize.rs b/src/librustc_trans/trans/monomorphize.rs index b736ec3ccf4ad..e28dd77d5e58e 100644 --- a/src/librustc_trans/trans/monomorphize.rs +++ b/src/librustc_trans/trans/monomorphize.rs @@ -337,7 +337,7 @@ pub fn normalize_associated_type<'tcx,T>(tcx: &ty::ctxt<'tcx>, value: &T) -> T result.repr(tcx), obligations.repr(tcx)); - let mut fulfill_cx = traits::FulfillmentContext::new(); + let mut fulfill_cx = traits::FulfillmentContext::new(true); for obligation in obligations { fulfill_cx.register_predicate_obligation(&infcx, obligation); } diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index b3267a5be495b..c5861be2716b2 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -45,7 +45,7 @@ pub fn compare_impl_method<'tcx>(tcx: &ty::ctxt<'tcx>, impl_trait_ref.repr(tcx)); let infcx = infer::new_infer_ctxt(tcx); - let mut fulfillment_cx = traits::FulfillmentContext::new(); + let mut fulfillment_cx = traits::FulfillmentContext::new(true); let trait_to_impl_substs = &impl_trait_ref.substs; @@ -422,7 +422,7 @@ pub fn compare_const_impl<'tcx>(tcx: &ty::ctxt<'tcx>, impl_trait_ref.repr(tcx)); let infcx = infer::new_infer_ctxt(tcx); - let mut fulfillment_cx = traits::FulfillmentContext::new(); + let mut fulfillment_cx = traits::FulfillmentContext::new(true); // The below is for the most part highly similar to the procedure // for methods above. It is simpler in many respects, especially diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 69f1b5091df46..9df0d4aa56b8a 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -386,7 +386,7 @@ impl<'a, 'tcx> Inherited<'a, 'tcx> { closure_tys: RefCell::new(DefIdMap()), closure_kinds: RefCell::new(DefIdMap()), fn_sig_map: RefCell::new(NodeMap()), - fulfillment_cx: RefCell::new(traits::FulfillmentContext::new()), + fulfillment_cx: RefCell::new(traits::FulfillmentContext::new(true)), deferred_call_resolutions: RefCell::new(DefIdMap()), deferred_cast_checks: RefCell::new(Vec::new()), } diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index f6a40eec5829c..cd7be46f9e012 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -539,7 +539,7 @@ impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> { } }; - let mut fulfill_cx = traits::FulfillmentContext::new(); + let mut fulfill_cx = traits::FulfillmentContext::new(true); // Register an obligation for `A: Trait`. let cause = traits::ObligationCause::misc(span, impl_did.node); From f0f13f86ef434809282bc5eca1bd04a23fdf88d2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 11 May 2015 17:02:56 -0400 Subject: [PATCH 2/4] 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 138231b599586..50536f586e7a8 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 1631a33588ce2..6a9ed1a8cf99e 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 307242d18dfef..211839fbd25f9 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 edd8f5ea6eaa9..62682e1a6fb84 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 0000000000000..0aefd0ae28845 --- /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::(); +} From 13cee1e1987936514266861a4afa231d7224ad93 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 15 Jun 2015 17:30:34 -0400 Subject: [PATCH 3/4] This new caching sometimes suppresses duplicate errors that occur in different parts of the crate, so modify the test cases that were relying on that to test distinct types etc. --- .../compile-fail/associated-types-path-2.rs | 1 - .../dst-object-from-unsized-type.rs | 7 ++--- .../compile-fail/kindck-impl-type-params.rs | 6 +++-- .../trait-bounds-on-structs-and-enums.rs | 4 +-- .../compile-fail/traits-negative-impls.rs | 27 ++++++++++++------- ...k-default-trait-impl-trait-where-clause.rs | 4 +-- src/test/compile-fail/unsized5.rs | 2 +- 7 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/test/compile-fail/associated-types-path-2.rs b/src/test/compile-fail/associated-types-path-2.rs index b9a62ff4e417a..e603cca7f384b 100644 --- a/src/test/compile-fail/associated-types-path-2.rs +++ b/src/test/compile-fail/associated-types-path-2.rs @@ -45,7 +45,6 @@ pub fn f1_uint_uint() { pub fn f1_uint_int() { f1(2u32, 4i32); //~^ ERROR the trait `Foo` is not implemented - //~| ERROR the trait `Foo` is not implemented } pub fn f2_int() { diff --git a/src/test/compile-fail/dst-object-from-unsized-type.rs b/src/test/compile-fail/dst-object-from-unsized-type.rs index b4fd45845f7a0..da667dcb1e882 100644 --- a/src/test/compile-fail/dst-object-from-unsized-type.rs +++ b/src/test/compile-fail/dst-object-from-unsized-type.rs @@ -12,6 +12,7 @@ trait Foo { fn foo(&self) {} } impl Foo for str {} +impl Foo for [u8] {} fn test1(t: &T) { let u: &Foo = t; @@ -28,9 +29,9 @@ fn test3() { //~^ ERROR `core::marker::Sized` is not implemented for the type `str` } -fn test4() { - let _: &Foo = "hi" as &Foo; - //~^ ERROR `core::marker::Sized` is not implemented for the type `str` +fn test4(x: &[u8]) { + let _: &Foo = x as &Foo; + //~^ ERROR `core::marker::Sized` is not implemented for the type `[u8]` } fn main() { } diff --git a/src/test/compile-fail/kindck-impl-type-params.rs b/src/test/compile-fail/kindck-impl-type-params.rs index 988a7837b59bf..3474a73b31fdc 100644 --- a/src/test/compile-fail/kindck-impl-type-params.rs +++ b/src/test/compile-fail/kindck-impl-type-params.rs @@ -50,8 +50,10 @@ fn foo2<'a>() { } fn foo3<'a>() { - let t: Box> = box S(marker::PhantomData); - let a: Box> = t; + struct Foo; // does not impl Copy + + let t: Box> = box S(marker::PhantomData); + let a: Box> = t; //~^ ERROR the trait `core::marker::Copy` is not implemented } diff --git a/src/test/compile-fail/trait-bounds-on-structs-and-enums.rs b/src/test/compile-fail/trait-bounds-on-structs-and-enums.rs index c13f7346b1178..988961e7fa14a 100644 --- a/src/test/compile-fail/trait-bounds-on-structs-and-enums.rs +++ b/src/test/compile-fail/trait-bounds-on-structs-and-enums.rs @@ -52,7 +52,7 @@ struct TupleLike( ); enum Enum { - DictionaryLike { field: Bar }, //~ ERROR not implemented + DictionaryLike { field: Bar }, //~ ERROR not implemented } trait PolyTrait @@ -62,7 +62,7 @@ trait PolyTrait struct Struct; -impl PolyTrait> for Struct { +impl PolyTrait> for Struct { //~^ ERROR not implemented } diff --git a/src/test/compile-fail/traits-negative-impls.rs b/src/test/compile-fail/traits-negative-impls.rs index 3ef760053c709..8dc977a8e490b 100644 --- a/src/test/compile-fail/traits-negative-impls.rs +++ b/src/test/compile-fail/traits-negative-impls.rs @@ -19,9 +19,6 @@ use std::marker::Send; struct Outer(T); -struct TestType; -impl !Send for TestType {} - struct Outer2(T); unsafe impl Sync for Outer2 {} @@ -30,29 +27,41 @@ fn is_send(_: T) {} fn is_sync(_: T) {} fn dummy() { + struct TestType; + impl !Send for TestType {} + Outer(TestType); - //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType` + //~^ ERROR the trait `core::marker::Send` is not implemented for the type `dummy::TestType` is_send(TestType); - //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType` + //~^ ERROR the trait `core::marker::Send` is not implemented for the type `dummy::TestType` is_send((8, TestType)); - //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType` + //~^ ERROR the trait `core::marker::Send` is not implemented for the type `dummy::TestType` } fn dummy2() { + struct TestType; + impl !Send for TestType {} + is_send(Box::new(TestType)); - //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType` + //~^ ERROR the trait `core::marker::Send` is not implemented for the type `dummy2::TestType` } fn dummy3() { + struct TestType; + impl !Send for TestType {} + is_send(Box::new(Outer2(TestType))); - //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType` + //~^ ERROR the trait `core::marker::Send` is not implemented for the type `dummy3::TestType` } fn main() { + struct TestType; + impl !Send for TestType {} + // This will complain about a missing Send impl because `Sync` is implement *just* // for T that are `Send`. Look at #20366 and #19950 is_sync(Outer2(TestType)); - //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType` + //~^ ERROR the trait `core::marker::Send` is not implemented for the type `main::TestType` } diff --git a/src/test/compile-fail/typeck-default-trait-impl-trait-where-clause.rs b/src/test/compile-fail/typeck-default-trait-impl-trait-where-clause.rs index 4f572e87639f0..8057ca56621c1 100644 --- a/src/test/compile-fail/typeck-default-trait-impl-trait-where-clause.rs +++ b/src/test/compile-fail/typeck-default-trait-impl-trait-where-clause.rs @@ -40,6 +40,6 @@ fn test() { } fn main() { - foo::(); - //~^ ERROR the trait `NotImplemented` is not implemented for the type `core::option::Option` + foo::(); + //~^ ERROR the trait `NotImplemented` is not implemented for the type `core::option::Option` } diff --git a/src/test/compile-fail/unsized5.rs b/src/test/compile-fail/unsized5.rs index dc10f795cd422..afa574bf85a8d 100644 --- a/src/test/compile-fail/unsized5.rs +++ b/src/test/compile-fail/unsized5.rs @@ -24,7 +24,7 @@ struct S3 { g: [usize] } struct S4 { - f: str, //~ ERROR `core::marker::Sized` is not implemented + f: [u8], //~ ERROR `core::marker::Sized` is not implemented g: usize } enum E { From 957935a002aa88adb0f1288a84a0933d06cc297d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 16 Jun 2015 12:08:37 -0400 Subject: [PATCH 4/4] Fix merge errors --- src/librustc/middle/ty.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 62682e1a6fb84..6662e3ef4b3b9 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -755,7 +755,8 @@ pub struct ctxt<'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 + /// added to this set when they mention only "global" names + /// (i.e., no type or lifetime parameters). pub fulfilled_predicates: RefCell>, /// Caches the representation hints for struct definitions. @@ -823,7 +824,7 @@ bitflags! { // true if there are "names" of types and regions and so forth // that are local to a particular fn - const HAS_LOCAL_NAMES = 1 << 8, + const HAS_LOCAL_NAMES = 1 << 9, const NEEDS_SUBST = TypeFlags::HAS_PARAMS.bits | TypeFlags::HAS_SELF.bits |