From 79f71f976a2c55794a120cb77cb8dfbf35bb0a25 Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Thu, 5 Apr 2018 12:29:18 -0500 Subject: [PATCH 1/8] Refactor overflow handling in traits::select to propagate overflow instead of aborting eagerly We store the obligation that caused the overflow as part of the OverflowError, and report it at the public API endpoints (rather than in the implementation internals). --- src/librustc/traits/error_reporting.rs | 5 + src/librustc/traits/mod.rs | 2 + src/librustc/traits/select.rs | 174 ++++++++++++++---------- src/librustc/traits/structural_impls.rs | 1 + 4 files changed, 113 insertions(+), 69 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 33abc0c7e15aa..686008cc1efd5 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -24,6 +24,7 @@ use super::{ SelectionContext, SelectionError, ObjectSafetyViolation, + Overflow, }; use errors::DiagnosticBuilder; @@ -830,6 +831,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } err.struct_error(self.tcx, span, "constant expression") } + + Overflow(_) => { + bug!("overflow should be handled before the `report_selection_error` path"); + } }; self.note_obligation_cause(&mut err, obligation); err.emit(); diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 728d9f1a0270b..6101d37956afd 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -349,6 +349,8 @@ pub enum SelectionError<'tcx> { ty::error::TypeError<'tcx>), TraitNotObjectSafe(DefId), ConstEvalFailure(ConstEvalErr<'tcx>), + // upon overflow, stores the obligation that hit the recursion limit + Overflow(TraitObligation<'tcx>), } pub struct FulfillmentError<'tcx> { diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index f43f5cf3e3ff4..e65bfa7a72ecd 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -22,7 +22,7 @@ use super::project; use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey}; use super::{PredicateObligation, TraitObligation, ObligationCause}; use super::{ObligationCauseCode, BuiltinDerivedObligation, ImplDerivedObligation}; -use super::{SelectionError, Unimplemented, OutputTypeParameterMismatch}; +use super::{SelectionError, Unimplemented, OutputTypeParameterMismatch, Overflow}; use super::{ObjectCastObligation, Obligation}; use super::TraitNotObjectSafe; use super::Selection; @@ -408,6 +408,17 @@ impl EvaluationResult { } } +#[derive(Clone, Debug, PartialEq, Eq)] +/// Indicates that trait evaluation caused overflow. Stores the obligation +/// that hit the recursion limit. +pub struct OverflowError<'tcx>(TraitObligation<'tcx>); + +impl<'tcx> From> for SelectionError<'tcx> { + fn from(OverflowError(o): OverflowError<'tcx>) -> SelectionError<'tcx> { + SelectionError::Overflow(o) + } +} + #[derive(Clone)] pub struct EvaluationCache<'tcx> { hashmap: RefCell, WithDepNode>> @@ -528,12 +539,21 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { assert!(!obligation.predicate.has_escaping_regions()); let stack = self.push_stack(TraitObligationStackList::empty(), obligation); - let ret = match self.candidate_from_obligation(&stack)? { - None => None, - Some(candidate) => Some(self.confirm_candidate(obligation, candidate)?) + + let candidate = match self.candidate_from_obligation(&stack) { + Err(SelectionError::Overflow(o)) => + self.infcx().report_overflow_error(&o, true), + Err(e) => { return Err(e); }, + Ok(None) => { return Ok(None); }, + Ok(Some(candidate)) => candidate }; - Ok(ret) + match self.confirm_candidate(obligation, candidate) { + Err(SelectionError::Overflow(o)) => + self.infcx().report_overflow_error(&o, true), + Err(e) => Err(e), + Ok(candidate) => Ok(Some(candidate)) + } } /////////////////////////////////////////////////////////////////////////// @@ -554,10 +574,12 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("evaluate_obligation({:?})", obligation); - self.probe(|this, _| { + match self.probe(|this, _| { this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation) - .may_apply() - }) + }) { + Ok(result) => result.may_apply(), + Err(OverflowError(o)) => self.infcx().report_overflow_error(&o, true) + } } /// Evaluates whether the obligation `obligation` can be satisfied, @@ -570,10 +592,12 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("evaluate_obligation_conservatively({:?})", obligation); - self.probe(|this, _| { + match self.probe(|this, _| { this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation) - == EvaluatedToOk - }) + }) { + Ok(result) => result == EvaluatedToOk, + Err(OverflowError(o)) => self.infcx().report_overflow_error(&o, true) + } } /// Evaluates the predicates in `predicates` recursively. Note that @@ -582,29 +606,29 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_predicates_recursively<'a,'o,I>(&mut self, stack: TraitObligationStackList<'o, 'tcx>, predicates: I) - -> EvaluationResult + -> Result> where I : IntoIterator>, 'tcx:'a { let mut result = EvaluatedToOk; for obligation in predicates { - let eval = self.evaluate_predicate_recursively(stack, obligation); + let eval = self.evaluate_predicate_recursively(stack, obligation)?; debug!("evaluate_predicate_recursively({:?}) = {:?}", obligation, eval); if let EvaluatedToErr = eval { // fast-path - EvaluatedToErr is the top of the lattice, // so we don't need to look on the other predicates. - return EvaluatedToErr; + return Ok(EvaluatedToErr); } else { result = cmp::max(result, eval); } } - result + Ok(result) } fn evaluate_predicate_recursively<'o>(&mut self, previous_stack: TraitObligationStackList<'o, 'tcx>, obligation: &PredicateObligation<'tcx>) - -> EvaluationResult + -> Result> { debug!("evaluate_predicate_recursively({:?})", obligation); @@ -620,11 +644,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // does this code ever run? match self.infcx.subtype_predicate(&obligation.cause, obligation.param_env, p) { Some(Ok(InferOk { obligations, .. })) => { - self.evaluate_predicates_recursively(previous_stack, &obligations); - EvaluatedToOk + self.evaluate_predicates_recursively(previous_stack, &obligations) }, - Some(Err(_)) => EvaluatedToErr, - None => EvaluatedToAmbig, + Some(Err(_)) => Ok(EvaluatedToErr), + None => Ok(EvaluatedToAmbig), } } @@ -636,21 +659,21 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { Some(obligations) => self.evaluate_predicates_recursively(previous_stack, obligations.iter()), None => - EvaluatedToAmbig, + Ok(EvaluatedToAmbig), } } ty::Predicate::TypeOutlives(..) | ty::Predicate::RegionOutlives(..) => { // we do not consider region relationships when // evaluating trait matches - EvaluatedToOk + Ok(EvaluatedToOk) } ty::Predicate::ObjectSafe(trait_def_id) => { if self.tcx().is_object_safe(trait_def_id) { - EvaluatedToOk + Ok(EvaluatedToOk) } else { - EvaluatedToErr + Ok(EvaluatedToErr) } } @@ -668,10 +691,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { result } Ok(None) => { - EvaluatedToAmbig + Ok(EvaluatedToAmbig) } Err(_) => { - EvaluatedToErr + Ok(EvaluatedToErr) } } } @@ -680,13 +703,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { match self.infcx.closure_kind(closure_def_id, closure_substs) { Some(closure_kind) => { if closure_kind.extends(kind) { - EvaluatedToOk + Ok(EvaluatedToOk) } else { - EvaluatedToErr + Ok(EvaluatedToErr) } } None => { - EvaluatedToAmbig + Ok(EvaluatedToAmbig) } } } @@ -707,16 +730,16 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { promoted: None }; match self.tcx().const_eval(param_env.and(cid)) { - Ok(_) => EvaluatedToOk, - Err(_) => EvaluatedToErr + Ok(_) => Ok(EvaluatedToOk), + Err(_) => Ok(EvaluatedToErr) } } else { - EvaluatedToErr + Ok(EvaluatedToErr) } } None => { // Inference variables still left in param_env or substs. - EvaluatedToAmbig + Ok(EvaluatedToAmbig) } } } @@ -726,7 +749,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_trait_predicate_recursively<'o>(&mut self, previous_stack: TraitObligationStackList<'o, 'tcx>, mut obligation: TraitObligation<'tcx>) - -> EvaluationResult + -> Result> { debug!("evaluate_trait_predicate_recursively({:?})", obligation); @@ -745,22 +768,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("CACHE HIT: EVAL({:?})={:?}", fresh_trait_ref, result); - return result; + return Ok(result); } let (result, dep_node) = self.in_task(|this| this.evaluate_stack(&stack)); + let result = result?; debug!("CACHE MISS: EVAL({:?})={:?}", fresh_trait_ref, result); self.insert_evaluation_cache(obligation.param_env, fresh_trait_ref, dep_node, result); - result + Ok(result) } fn evaluate_stack<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) - -> EvaluationResult + -> Result> { // In intercrate mode, whenever any of the types are unbound, // there can always be an impl. Even if there are no impls in @@ -815,7 +839,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { } } } - return EvaluatedToAmbig; + return Ok(EvaluatedToAmbig); } if unbound_input_types && stack.iter().skip(1).any( @@ -825,7 +849,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { { debug!("evaluate_stack({:?}) --> unbound argument, recursive --> giving up", stack.fresh_trait_ref); - return EvaluatedToUnknown; + return Ok(EvaluatedToUnknown); } // If there is any previous entry on the stack that precisely @@ -860,18 +884,19 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { if self.coinductive_match(cycle) { debug!("evaluate_stack({:?}) --> recursive, coinductive", stack.fresh_trait_ref); - return EvaluatedToOk; + return Ok(EvaluatedToOk); } else { debug!("evaluate_stack({:?}) --> recursive, inductive", stack.fresh_trait_ref); - return EvaluatedToRecur; + return Ok(EvaluatedToRecur); } } match self.candidate_from_obligation(stack) { Ok(Some(c)) => self.evaluate_candidate(stack, &c), - Ok(None) => EvaluatedToAmbig, - Err(..) => EvaluatedToErr + Ok(None) => Ok(EvaluatedToAmbig), + Err(Overflow(o)) => Err(OverflowError(o)), + Err(..) => Ok(EvaluatedToErr) } } @@ -909,7 +934,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_candidate<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>, candidate: &SelectionCandidate<'tcx>) - -> EvaluationResult + -> Result> { debug!("evaluate_candidate: depth={} candidate={:?}", stack.obligation.recursion_depth, candidate); @@ -921,12 +946,12 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { stack.list(), selection.nested_obligations().iter()) } - Err(..) => EvaluatedToErr + Err(..) => Ok(EvaluatedToErr) } - }); + })?; debug!("evaluate_candidate: depth={} result={:?}", stack.obligation.recursion_depth, result); - result + Ok(result) } fn check_evaluation_cache(&self, @@ -1000,7 +1025,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // not update) the cache. let recursion_limit = *self.infcx.tcx.sess.recursion_limit.get(); if stack.obligation.recursion_depth >= recursion_limit { - self.infcx().report_overflow_error(&stack.obligation, true); + return Err(Overflow(stack.obligation.clone())); } // Check the cache. Note that we skolemize the trait-ref @@ -1081,9 +1106,15 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("evaluate_stack: intercrate_ambiguity_causes is some"); // Heuristics: show the diagnostics when there are no candidates in crate. if let Ok(candidate_set) = self.assemble_candidates(stack) { - if !candidate_set.ambiguous && candidate_set.vec.iter().all(|c| { - !self.evaluate_candidate(stack, &c).may_apply() - }) { + let no_candidates_apply = + candidate_set + .vec + .iter() + .map(|c| self.evaluate_candidate(stack, &c)) + .collect::, OverflowError<'_>>>()? + .iter() + .all(|r| !r.may_apply()); + if !candidate_set.ambiguous && no_candidates_apply { let trait_ref = stack.obligation.predicate.skip_binder().trait_ref; let self_ty = trait_ref.self_ty(); let trait_desc = trait_ref.to_string(); @@ -1151,18 +1182,21 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { } // Winnow, but record the exact outcome of evaluation, which - // is needed for specialization. - let mut candidates: Vec<_> = candidates.into_iter().filter_map(|c| { - let eval = self.evaluate_candidate(stack, &c); - if eval.may_apply() { - Some(EvaluatedCandidate { + // is needed for specialization. Propagate overflow if it occurs. + let candidates: Result>, _> = candidates + .into_iter() + .map(|c| match self.evaluate_candidate(stack, &c) { + Ok(eval) if eval.may_apply() => Ok(Some(EvaluatedCandidate { candidate: c, evaluation: eval, - }) - } else { - None - } - }).collect(); + })), + Ok(_) => Ok(None), + Err(OverflowError(o)) => Err(Overflow(o)), + }) + .collect(); + + let mut candidates: Vec = + candidates?.into_iter().filter_map(|c| c).collect(); // If there are STILL multiple candidate, we can further // reduce the list by dropping duplicates -- including @@ -1537,12 +1571,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { let matching_bounds = all_bounds.filter(|p| p.def_id() == stack.obligation.predicate.def_id()); - let matching_bounds = - matching_bounds.filter( - |bound| self.evaluate_where_clause(stack, bound.clone()).may_apply()); - - let param_candidates = - matching_bounds.map(|bound| ParamCandidate(bound)); + // keep only those bounds which may apply, and propagate overflow if it occurs + let mut param_candidates = vec![]; + for bound in matching_bounds { + let wc = self.evaluate_where_clause(stack, bound.clone())?; + if wc.may_apply() { + param_candidates.push(ParamCandidate(bound)); + } + } candidates.vec.extend(param_candidates); @@ -1552,14 +1588,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_where_clause<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>, where_clause_trait_ref: ty::PolyTraitRef<'tcx>) - -> EvaluationResult + -> Result> { self.probe(move |this, _| { match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) { Ok(obligations) => { this.evaluate_predicates_recursively(stack.list(), obligations.iter()) } - Err(()) => EvaluatedToErr + Err(()) => Ok(EvaluatedToErr) } }) } diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 1e3e4160de196..c124cd7942a68 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -177,6 +177,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::SelectionError<'a> { super::ConstEvalFailure(ref err) => { tcx.lift(err).map(super::ConstEvalFailure) } + super::Overflow(_) => bug!() // FIXME: ape ConstEvalFailure? } } } From 3ab3a9f50994b6e15de7fe92acf5a196ea6575c9 Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Thu, 8 Mar 2018 18:30:37 -0600 Subject: [PATCH 2/8] Create a canonical trait query for `evaluate_obligation` --- src/librustc/dep_graph/dep_node.rs | 4 +- src/librustc/traits/mod.rs | 2 +- .../traits/query/evaluate_obligation.rs | 54 +++++++++++++++++++ src/librustc/traits/query/mod.rs | 4 ++ src/librustc/traits/select.rs | 33 ++++++++---- src/librustc/ty/maps/config.rs | 8 ++- src/librustc/ty/maps/keys.rs | 12 ++++- src/librustc/ty/maps/mod.rs | 10 +++- src/librustc/ty/maps/plumbing.rs | 1 + src/librustc_traits/evaluate_obligation.rs | 40 ++++++++++++++ src/librustc_traits/lib.rs | 2 + 11 files changed, 155 insertions(+), 15 deletions(-) create mode 100644 src/librustc/traits/query/evaluate_obligation.rs create mode 100644 src/librustc_traits/evaluate_obligation.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 18bf54297afc6..e4f432e7caf49 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -70,7 +70,8 @@ use rustc_data_structures::stable_hasher::{StableHasher, HashStable}; use std::fmt; use std::hash::Hash; use syntax_pos::symbol::InternedString; -use traits::query::{CanonicalProjectionGoal, CanonicalTyGoal}; +use traits::query::{CanonicalProjectionGoal, + CanonicalTyGoal, CanonicalPredicateGoal}; use ty::{TyCtxt, Instance, InstanceDef, ParamEnv, ParamEnvAnd, PolyTraitRef, Ty}; use ty::subst::Substs; @@ -643,6 +644,7 @@ define_dep_nodes!( <'tcx> [] NormalizeProjectionTy(CanonicalProjectionGoal<'tcx>), [] NormalizeTyAfterErasingRegions(ParamEnvAnd<'tcx, Ty<'tcx>>), [] DropckOutlives(CanonicalTyGoal<'tcx>), + [] EvaluateObligation(CanonicalPredicateGoal<'tcx>), [] SubstituteNormalizeAndTestPredicates { key: (DefId, &'tcx Substs<'tcx>) }, diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 6101d37956afd..031a3677aa330 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -41,7 +41,7 @@ pub use self::object_safety::ObjectSafetyViolation; pub use self::object_safety::MethodViolationCode; pub use self::on_unimplemented::{OnUnimplementedDirective, OnUnimplementedNote}; pub use self::select::{EvaluationCache, SelectionContext, SelectionCache}; -pub use self::select::IntercrateAmbiguityCause; +pub use self::select::{EvaluationResult, IntercrateAmbiguityCause, OverflowError}; pub use self::specialize::{OverlapError, specialization_graph, translate_substs}; pub use self::specialize::{SpecializesCache, find_associated_item}; pub use self::engine::TraitEngine; diff --git a/src/librustc/traits/query/evaluate_obligation.rs b/src/librustc/traits/query/evaluate_obligation.rs new file mode 100644 index 0000000000000..88c51d006db06 --- /dev/null +++ b/src/librustc/traits/query/evaluate_obligation.rs @@ -0,0 +1,54 @@ +// Copyright 2018 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. + +use infer::InferCtxt; +use infer::canonical::{Canonical, Canonicalize}; +use traits::{EvaluationResult, PredicateObligation}; +use traits::query::CanonicalPredicateGoal; +use ty::{ParamEnvAnd, Predicate, TyCtxt}; + +impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { + /// Evaluates whether the predicate can be satisfied (by any means) + /// in the given `ParamEnv`. + pub fn predicate_may_hold( + &self, + obligation: &PredicateObligation<'tcx>, + ) -> bool { + let (c_pred, _) = + self.canonicalize_query(&obligation.param_env.and(obligation.predicate)); + + self.tcx.global_tcx().evaluate_obligation(c_pred).may_apply() + } + + /// Evaluates whether the predicate can be satisfied in the given + /// `ParamEnv`, and returns `false` if not certain. However, this is + /// not entirely accurate if inference variables are involved. + pub fn predicate_must_hold( + &self, + obligation: &PredicateObligation<'tcx>, + ) -> bool { + let (c_pred, _) = + self.canonicalize_query(&obligation.param_env.and(obligation.predicate)); + + self.tcx.global_tcx().evaluate_obligation(c_pred) == + EvaluationResult::EvaluatedToOk + } +} + +impl<'gcx: 'tcx, 'tcx> Canonicalize<'gcx, 'tcx> for ParamEnvAnd<'tcx, Predicate<'tcx>> { + type Canonicalized = CanonicalPredicateGoal<'gcx>; + + fn intern( + _gcx: TyCtxt<'_, 'gcx, 'gcx>, + value: Canonical<'gcx, Self::Lifted>, + ) -> Self::Canonicalized { + value + } +} diff --git a/src/librustc/traits/query/mod.rs b/src/librustc/traits/query/mod.rs index f1f9256f82537..096633ddab2f7 100644 --- a/src/librustc/traits/query/mod.rs +++ b/src/librustc/traits/query/mod.rs @@ -19,6 +19,7 @@ use infer::canonical::Canonical; use ty::{self, Ty}; pub mod dropck_outlives; +pub mod evaluate_obligation; pub mod normalize; pub mod normalize_erasing_regions; @@ -27,6 +28,9 @@ pub type CanonicalProjectionGoal<'tcx> = pub type CanonicalTyGoal<'tcx> = Canonical<'tcx, ty::ParamEnvAnd<'tcx, Ty<'tcx>>>; +pub type CanonicalPredicateGoal<'tcx> = + Canonical<'tcx, ty::ParamEnvAnd<'tcx, ty::Predicate<'tcx>>>; + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub struct NoSolution; diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index e65bfa7a72ecd..3dedfcd357eb6 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -319,7 +319,7 @@ enum BuiltinImplConditions<'tcx> { /// all the "potential success" candidates can potentially succeed, /// so they are no-ops when unioned with a definite error, and within /// the categories it's easy to see that the unions are correct. -enum EvaluationResult { +pub enum EvaluationResult { /// Evaluation successful EvaluatedToOk, /// Evaluation is known to be ambiguous - it *might* hold for some @@ -385,7 +385,7 @@ enum EvaluationResult { } impl EvaluationResult { - fn may_apply(self) -> bool { + pub fn may_apply(self) -> bool { match self { EvaluatedToOk | EvaluatedToAmbig | @@ -408,10 +408,18 @@ impl EvaluationResult { } } +impl_stable_hash_for!(enum self::EvaluationResult { + EvaluatedToOk, + EvaluatedToAmbig, + EvaluatedToUnknown, + EvaluatedToRecur, + EvaluatedToErr +}); + #[derive(Clone, Debug, PartialEq, Eq)] /// Indicates that trait evaluation caused overflow. Stores the obligation /// that hit the recursion limit. -pub struct OverflowError<'tcx>(TraitObligation<'tcx>); +pub struct OverflowError<'tcx>(pub TraitObligation<'tcx>); impl<'tcx> From> for SelectionError<'tcx> { fn from(OverflowError(o): OverflowError<'tcx>) -> SelectionError<'tcx> { @@ -574,9 +582,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("evaluate_obligation({:?})", obligation); - match self.probe(|this, _| { - this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation) - }) { + match self.evaluate_obligation_recursively(obligation) { Ok(result) => result.may_apply(), Err(OverflowError(o)) => self.infcx().report_overflow_error(&o, true) } @@ -592,14 +598,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("evaluate_obligation_conservatively({:?})", obligation); - match self.probe(|this, _| { - this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation) - }) { + match self.evaluate_obligation_recursively(obligation) { Ok(result) => result == EvaluatedToOk, Err(OverflowError(o)) => self.infcx().report_overflow_error(&o, true) } } + /// Evaluates whether the obligation `obligation` can be satisfied and returns + /// an `EvaluationResult`. + pub fn evaluate_obligation_recursively(&mut self, + obligation: &PredicateObligation<'tcx>) + -> Result> + { + self.probe(|this, _| { + this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation) + }) + } + /// Evaluates the predicates in `predicates` recursively. Note that /// this applies projections in the predicates, and therefore /// is run within an inference probe. diff --git a/src/librustc/ty/maps/config.rs b/src/librustc/ty/maps/config.rs index 735fe06560f5f..1fd0632580cfc 100644 --- a/src/librustc/ty/maps/config.rs +++ b/src/librustc/ty/maps/config.rs @@ -11,7 +11,7 @@ use dep_graph::SerializedDepNodeIndex; use hir::def_id::{CrateNum, DefId, DefIndex}; use mir::interpret::{GlobalId}; -use traits::query::{CanonicalProjectionGoal, CanonicalTyGoal}; +use traits::query::{CanonicalPredicateGoal, CanonicalProjectionGoal, CanonicalTyGoal}; use ty::{self, ParamEnvAnd, Ty, TyCtxt}; use ty::subst::Substs; use ty::maps::queries; @@ -73,6 +73,12 @@ impl<'tcx> QueryDescription<'tcx> for queries::normalize_ty_after_erasing_region } } +impl<'tcx> QueryDescription<'tcx> for queries::evaluate_obligation<'tcx> { + fn describe(_tcx: TyCtxt, goal: CanonicalPredicateGoal<'tcx>) -> String { + format!("evaluating trait selection obligation `{}`", goal.value.value) + } +} + impl<'tcx> QueryDescription<'tcx> for queries::is_copy_raw<'tcx> { fn describe(_tcx: TyCtxt, env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> String { format!("computing whether `{}` is `Copy`", env.value) diff --git a/src/librustc/ty/maps/keys.rs b/src/librustc/ty/maps/keys.rs index 72f2cb49abc0e..da29f23589e85 100644 --- a/src/librustc/ty/maps/keys.rs +++ b/src/librustc/ty/maps/keys.rs @@ -11,7 +11,7 @@ //! Defines the set of legal keys that can be used in queries. use hir::def_id::{CrateNum, DefId, LOCAL_CRATE, DefIndex}; -use traits::query::{CanonicalProjectionGoal, CanonicalTyGoal}; +use traits::query::{CanonicalPredicateGoal, CanonicalProjectionGoal, CanonicalTyGoal}; use ty::{self, Ty, TyCtxt}; use ty::subst::Substs; use ty::fast_reject::SimplifiedType; @@ -200,3 +200,13 @@ impl<'tcx> Key for CanonicalTyGoal<'tcx> { DUMMY_SP } } + +impl<'tcx> Key for CanonicalPredicateGoal<'tcx> { + fn map_crate(&self) -> CrateNum { + LOCAL_CRATE + } + + fn default_span(&self, _tcx: TyCtxt) -> Span { + DUMMY_SP + } +} diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs index f5cb3643de832..9343eccd38e9f 100644 --- a/src/librustc/ty/maps/mod.rs +++ b/src/librustc/ty/maps/mod.rs @@ -32,8 +32,9 @@ use mir; use mir::interpret::{GlobalId}; use session::{CompileResult, CrateDisambiguator}; use session::config::OutputFilenames; -use traits::Vtable; -use traits::query::{CanonicalProjectionGoal, CanonicalTyGoal, NoSolution}; +use traits::{self, Vtable}; +use traits::query::{CanonicalPredicateGoal, CanonicalProjectionGoal, + CanonicalTyGoal, NoSolution}; use traits::query::dropck_outlives::{DtorckConstraint, DropckOutlivesResult}; use traits::query::normalize::NormalizationResult; use traits::specialization_graph; @@ -433,6 +434,11 @@ define_maps! { <'tcx> NoSolution, >, + /// Do not call this query directly: invoke `infcx.predicate_may_hold()` or + /// `infcx.predicate_must_hold()` instead. + [] fn evaluate_obligation: + EvaluateObligation(CanonicalPredicateGoal<'tcx>) -> traits::EvaluationResult, + [] fn substitute_normalize_and_test_predicates: substitute_normalize_and_test_predicates_node((DefId, &'tcx Substs<'tcx>)) -> bool, diff --git a/src/librustc/ty/maps/plumbing.rs b/src/librustc/ty/maps/plumbing.rs index 61a4eb5853102..1cde745d4d87f 100644 --- a/src/librustc/ty/maps/plumbing.rs +++ b/src/librustc/ty/maps/plumbing.rs @@ -977,6 +977,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, DepKind::NormalizeProjectionTy | DepKind::NormalizeTyAfterErasingRegions | DepKind::DropckOutlives | + DepKind::EvaluateObligation | DepKind::SubstituteNormalizeAndTestPredicates | DepKind::InstanceDefSizeEstimate | DepKind::ProgramClausesForEnv | diff --git a/src/librustc_traits/evaluate_obligation.rs b/src/librustc_traits/evaluate_obligation.rs new file mode 100644 index 0000000000000..f43733f46377e --- /dev/null +++ b/src/librustc_traits/evaluate_obligation.rs @@ -0,0 +1,40 @@ +// Copyright 2018 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. + +use rustc::traits::{EvaluationResult, Obligation, ObligationCause, + OverflowError, SelectionContext}; +use rustc::traits::query::CanonicalPredicateGoal; +use rustc::ty::{ParamEnvAnd, TyCtxt}; +use syntax::codemap::DUMMY_SP; + +crate fn evaluate_obligation<'tcx>( + tcx: TyCtxt<'_, 'tcx, 'tcx>, + goal: CanonicalPredicateGoal<'tcx>, +) -> EvaluationResult { + tcx.infer_ctxt().enter(|ref infcx| { + let ( + ParamEnvAnd { + param_env, + value: predicate, + }, + _canonical_inference_vars, + ) = infcx.instantiate_canonical_with_fresh_inference_vars(DUMMY_SP, &goal); + + let mut selcx = SelectionContext::new(&infcx); + let obligation = Obligation::new(ObligationCause::dummy(), param_env, predicate); + + match selcx.evaluate_obligation_recursively(&obligation) { + Ok(result) => result, + Err(OverflowError(o)) => { + infcx.report_overflow_error(&o, true) + } + } + }) +} diff --git a/src/librustc_traits/lib.rs b/src/librustc_traits/lib.rs index 90c870096e179..7f18fac2db5a3 100644 --- a/src/librustc_traits/lib.rs +++ b/src/librustc_traits/lib.rs @@ -22,6 +22,7 @@ extern crate syntax; extern crate syntax_pos; mod dropck_outlives; +mod evaluate_obligation; mod normalize_projection_ty; mod normalize_erasing_regions; mod util; @@ -38,6 +39,7 @@ pub fn provide(p: &mut Providers) { normalize_erasing_regions::normalize_ty_after_erasing_regions, program_clauses_for: lowering::program_clauses_for, program_clauses_for_env: lowering::program_clauses_for_env, + evaluate_obligation: evaluate_obligation::evaluate_obligation, ..*p }; } From bc16b8e92a8e7dcf14b9c94b9248b5ad6957b1f4 Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Sun, 8 Apr 2018 01:56:27 -0500 Subject: [PATCH 3/8] Port existing callers of `evaluate_obligation` to the new canonical trait query Except the one in coherence, which needs support for intercrate mode. --- src/librustc/traits/coherence.rs | 3 +++ src/librustc/traits/error_reporting.rs | 8 +++----- src/librustc/traits/fulfill.rs | 2 +- src/librustc/traits/mod.rs | 3 +-- src/librustc_typeck/check/autoderef.rs | 4 ++-- src/librustc_typeck/check/method/mod.rs | 3 +-- src/librustc_typeck/check/method/probe.rs | 4 ++-- src/librustc_typeck/check/method/suggest.rs | 4 ++-- src/test/ui/impl-trait/auto-trait-leak.stderr | 2 ++ 9 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 31f8af1f96872..2e271669ed77d 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -155,6 +155,9 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, predicate: p }) .chain(obligations) .find(|o| !selcx.evaluate_obligation(o)); + // FIXME: the call to `selcx.evaluate_obligation` above should be ported + // to the canonical trait query form, `infcx.predicate_may_hold`, once + // the new system supports intercrate mode (which coherence needs). if let Some(failing_obligation) = opt_failing_obligation { debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 686008cc1efd5..98684825d91f7 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -660,8 +660,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { predicate: ty::Predicate::Trait(predicate), .. obligation.clone() }; - let mut selcx = SelectionContext::new(self); - if selcx.evaluate_obligation(&unit_obligation) { + if self.predicate_may_hold(&unit_obligation) { err.note("the trait is implemented for `()`. \ Possibly this error has been caused by changes to \ Rust's type-inference algorithm \ @@ -877,7 +876,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { .count(); let mut trait_type = trait_ref.self_ty(); - let mut selcx = SelectionContext::new(self); for refs_remaining in 0..refs_number { if let ty::TypeVariants::TyRef(_, ty::TypeAndMut{ ty: t_type, mutbl: _ }) = @@ -891,7 +889,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { obligation.param_env, new_trait_ref.to_predicate()); - if selcx.evaluate_obligation(&new_obligation) { + if self.predicate_may_hold(&new_obligation) { let sp = self.tcx.sess.codemap() .span_take_while(span, |c| c.is_whitespace() || *c == '&'); @@ -1327,7 +1325,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { cleaned_pred.to_predicate() ); - selcx.evaluate_obligation(&obligation) + self.predicate_may_hold(&obligation) }) } diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index 1c091d68a2ef1..7bd0148b3c464 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -333,7 +333,7 @@ fn process_predicate<'a, 'gcx, 'tcx>( if data.is_global() { // no type variables present, can use evaluation for better caching. // FIXME: consider caching errors too. - if selcx.evaluate_obligation_conservatively(&obligation) { + if selcx.infcx().predicate_must_hold(&obligation) { debug!("selecting trait `{:?}` at depth {} evaluated to holds", data, obligation.recursion_depth); return Ok(Some(vec![])) diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 031a3677aa330..f09981dd8fcab 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -552,8 +552,7 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx predicate: trait_ref.to_predicate(), }; - let result = SelectionContext::new(infcx) - .evaluate_obligation_conservatively(&obligation); + let result = infcx.predicate_must_hold(&obligation); debug!("type_known_to_meet_ty={:?} bound={} => {:?}", ty, infcx.tcx.item_path_str(def_id), result); diff --git a/src/librustc_typeck/check/autoderef.rs b/src/librustc_typeck/check/autoderef.rs index a87058d1fa593..3199ff17ae926 100644 --- a/src/librustc_typeck/check/autoderef.rs +++ b/src/librustc_typeck/check/autoderef.rs @@ -120,15 +120,15 @@ impl<'a, 'gcx, 'tcx> Autoderef<'a, 'gcx, 'tcx> { let cause = traits::ObligationCause::misc(self.span, self.fcx.body_id); - let mut selcx = traits::SelectionContext::new(self.fcx); let obligation = traits::Obligation::new(cause.clone(), self.fcx.param_env, trait_ref.to_predicate()); - if !selcx.evaluate_obligation(&obligation) { + if !self.fcx.predicate_may_hold(&obligation) { debug!("overloaded_deref_ty: cannot match obligation"); return None; } + let mut selcx = traits::SelectionContext::new(self.fcx); let normalized = traits::normalize_projection_type(&mut selcx, self.fcx.param_env, ty::ProjectionTy::from_ref_and_name( diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 5f55ee6163b53..5f904a9419b1d 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -277,8 +277,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { poly_trait_ref.to_predicate()); // Now we want to know if this can be matched - let mut selcx = traits::SelectionContext::new(self); - if !selcx.evaluate_obligation(&obligation) { + if !self.predicate_may_hold(&obligation) { debug!("--> Cannot match obligation"); return None; // Cannot be matched, no such method resolution is possible. } diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index b41a6dcf384dd..476ae6809737a 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -1173,7 +1173,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let predicate = trait_ref.to_predicate(); let obligation = traits::Obligation::new(cause.clone(), self.param_env, predicate); - if !selcx.evaluate_obligation(&obligation) { + if !self.predicate_may_hold(&obligation) { if self.probe(|_| self.select_trait_candidate(trait_ref).is_err()) { // This candidate's primary obligation doesn't even // select - don't bother registering anything in @@ -1201,7 +1201,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { // Evaluate those obligations to see if they might possibly hold. for o in candidate_obligations.into_iter().chain(sub_obligations) { let o = self.resolve_type_vars_if_possible(&o); - if !selcx.evaluate_obligation(&o) { + if !self.predicate_may_hold(&o) { result = ProbeResult::NoMatch; if let &ty::Predicate::Trait(ref pred) = &o.predicate { possibly_unsatisfied_predicates.push(pred.skip_binder().trait_ref); diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index d8907866467ba..2dc7c7fe71a89 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -19,7 +19,7 @@ use hir::def::Def; use hir::def_id::{CRATE_DEF_INDEX, DefId}; use middle::lang_items::FnOnceTraitLangItem; use namespace::Namespace; -use rustc::traits::{Obligation, SelectionContext}; +use rustc::traits::Obligation; use util::nodemap::FxHashSet; use syntax::ast; @@ -65,7 +65,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.body_id, self.param_env, poly_trait_ref.to_predicate()); - SelectionContext::new(self).evaluate_obligation(&obligation) + self.predicate_may_hold(&obligation) }) }) } diff --git a/src/test/ui/impl-trait/auto-trait-leak.stderr b/src/test/ui/impl-trait/auto-trait-leak.stderr index 3b20451b10215..efa9a58d63310 100644 --- a/src/test/ui/impl-trait/auto-trait-leak.stderr +++ b/src/test/ui/impl-trait/auto-trait-leak.stderr @@ -34,6 +34,7 @@ error[E0391]: cycle detected when processing `cycle1` LL | fn cycle1() -> impl Clone { | ^^^^^^^^^^^^^^^^^^^^^^^^^ | +note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`... note: ...which requires processing `cycle2::{{impl-Trait}}`... --> $DIR/auto-trait-leak.rs:49:16 | @@ -44,6 +45,7 @@ note: ...which requires processing `cycle2`... | LL | fn cycle2() -> impl Clone { | ^^^^^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`... note: ...which requires processing `cycle1::{{impl-Trait}}`... --> $DIR/auto-trait-leak.rs:42:16 | From 3dd26b8a3ef9e10dd92f1c958e97493dfbd8788d Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Sun, 8 Apr 2018 02:04:46 -0500 Subject: [PATCH 4/8] Simplify trait selector's evaluation API slightly --- src/librustc/traits/coherence.rs | 4 ++-- src/librustc/traits/select.rs | 24 ++++-------------------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 2e271669ed77d..5a626e7b82cc4 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -154,8 +154,8 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, recursion_depth: 0, predicate: p }) .chain(obligations) - .find(|o| !selcx.evaluate_obligation(o)); - // FIXME: the call to `selcx.evaluate_obligation` above should be ported + .find(|o| !selcx.predicate_may_hold_fatal(o)); + // FIXME: the call to `selcx.predicate_may_hold_fatal` above should be ported // to the canonical trait query form, `infcx.predicate_may_hold`, once // the new system supports intercrate mode (which coherence needs). diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 3dedfcd357eb6..aae21e62051ad 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -575,11 +575,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // we can be sure it does not. /// Evaluates whether the obligation `obligation` can be satisfied (by any means). - pub fn evaluate_obligation(&mut self, - obligation: &PredicateObligation<'tcx>) - -> bool + pub fn predicate_may_hold_fatal(&mut self, + obligation: &PredicateObligation<'tcx>) + -> bool { - debug!("evaluate_obligation({:?})", + debug!("predicate_may_hold_fatal({:?})", obligation); match self.evaluate_obligation_recursively(obligation) { @@ -588,22 +588,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { } } - /// Evaluates whether the obligation `obligation` can be satisfied, - /// and returns `false` if not certain. However, this is not entirely - /// accurate if inference variables are involved. - pub fn evaluate_obligation_conservatively(&mut self, - obligation: &PredicateObligation<'tcx>) - -> bool - { - debug!("evaluate_obligation_conservatively({:?})", - obligation); - - match self.evaluate_obligation_recursively(obligation) { - Ok(result) => result == EvaluatedToOk, - Err(OverflowError(o)) => self.infcx().report_overflow_error(&o, true) - } - } - /// Evaluates whether the obligation `obligation` can be satisfied and returns /// an `EvaluationResult`. pub fn evaluate_obligation_recursively(&mut self, From e5535fc7ddf73f03c3adf2ea4097a171b2f5587f Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Thu, 19 Apr 2018 02:28:03 -0500 Subject: [PATCH 5/8] Introduce trait query mode and use it to set overflow error handling policy in traits::select --- src/librustc/traits/mod.rs | 13 +++++ src/librustc/traits/select.rs | 56 +++++++++++++++++----- src/librustc_traits/evaluate_obligation.rs | 4 +- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index f09981dd8fcab..6a8a000b58a10 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -74,6 +74,19 @@ pub enum IntercrateMode { Fixed } +// The mode that trait queries run in +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum TraitQueryMode { + // Standard/un-canonicalized queries get accurate + // spans etc. passed in and hence can do reasonable + // error reporting on their own. + Standard, + // Canonicalized queries get dummy spans and hence + // must generally propagate errors to + // pre-canonicalization callsites. + Canonical, +} + /// An `Obligation` represents some trait reference (e.g. `int:Eq`) for /// which the vtable must be found. The process of finding a vtable is /// called "resolving" the `Obligation`. This process consists of diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index aae21e62051ad..342b163b2f72b 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -17,7 +17,7 @@ use self::EvaluationResult::*; use super::coherence::{self, Conflict}; use super::DerivedObligationCause; -use super::IntercrateMode; +use super::{IntercrateMode, TraitQueryMode}; use super::project; use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey}; use super::{PredicateObligation, TraitObligation, ObligationCause}; @@ -87,7 +87,12 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { /// Controls whether or not to filter out negative impls when selecting. /// This is used in librustdoc to distinguish between the lack of an impl /// and a negative impl - allow_negative_impls: bool + allow_negative_impls: bool, + + /// The mode that trait queries run in, which informs our error handling + /// policy. In essence, canonicalized queries need their errors propagated + /// rather than immediately reported because we do not have accurate spans. + query_mode: TraitQueryMode, } #[derive(Clone, Debug)] @@ -440,6 +445,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { intercrate: None, intercrate_ambiguity_causes: None, allow_negative_impls: false, + query_mode: TraitQueryMode::Standard, } } @@ -452,6 +458,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { intercrate: Some(mode), intercrate_ambiguity_causes: None, allow_negative_impls: false, + query_mode: TraitQueryMode::Standard, } } @@ -464,6 +471,20 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { intercrate: None, intercrate_ambiguity_causes: None, allow_negative_impls, + query_mode: TraitQueryMode::Standard, + } + } + + pub fn with_query_mode(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>, + query_mode: TraitQueryMode) -> SelectionContext<'cx, 'gcx, 'tcx> { + debug!("with_query_mode({:?})", query_mode); + SelectionContext { + infcx, + freshener: infcx.freshener(), + intercrate: None, + intercrate_ambiguity_causes: None, + allow_negative_impls: false, + query_mode, } } @@ -548,17 +569,20 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { let stack = self.push_stack(TraitObligationStackList::empty(), obligation); + // `select` is currently only called in standard query mode + assert!(self.query_mode == TraitQueryMode::Standard); + let candidate = match self.candidate_from_obligation(&stack) { - Err(SelectionError::Overflow(o)) => - self.infcx().report_overflow_error(&o, true), + Err(SelectionError::Overflow(_)) => + bug!("Overflow should be caught earlier in standard query mode"), Err(e) => { return Err(e); }, Ok(None) => { return Ok(None); }, Ok(Some(candidate)) => candidate }; match self.confirm_candidate(obligation, candidate) { - Err(SelectionError::Overflow(o)) => - self.infcx().report_overflow_error(&o, true), + Err(SelectionError::Overflow(_)) => + bug!("Overflow should be caught earlier in standard query mode"), Err(e) => Err(e), Ok(candidate) => Ok(Some(candidate)) } @@ -582,10 +606,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("predicate_may_hold_fatal({:?})", obligation); - match self.evaluate_obligation_recursively(obligation) { - Ok(result) => result.may_apply(), - Err(OverflowError(o)) => self.infcx().report_overflow_error(&o, true) - } + // This fatal query is a stopgap that should only be used in standard mode, + // where we do not expect overflow to be propagated. + assert!(self.query_mode == TraitQueryMode::Standard); + + self.evaluate_obligation_recursively(obligation) + .expect("Overflow should be caught earlier in standard query mode") + .may_apply() } /// Evaluates whether the obligation `obligation` can be satisfied and returns @@ -1024,7 +1051,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // not update) the cache. let recursion_limit = *self.infcx.tcx.sess.recursion_limit.get(); if stack.obligation.recursion_depth >= recursion_limit { - return Err(Overflow(stack.obligation.clone())); + match self.query_mode { + TraitQueryMode::Standard => { + self.infcx().report_overflow_error(&stack.obligation, true); + }, + TraitQueryMode::Canonical => { + return Err(Overflow(stack.obligation.clone())); + }, + } } // Check the cache. Note that we skolemize the trait-ref diff --git a/src/librustc_traits/evaluate_obligation.rs b/src/librustc_traits/evaluate_obligation.rs index f43733f46377e..a9cf35c4d70d4 100644 --- a/src/librustc_traits/evaluate_obligation.rs +++ b/src/librustc_traits/evaluate_obligation.rs @@ -9,7 +9,7 @@ // except according to those terms. use rustc::traits::{EvaluationResult, Obligation, ObligationCause, - OverflowError, SelectionContext}; + OverflowError, SelectionContext, TraitQueryMode}; use rustc::traits::query::CanonicalPredicateGoal; use rustc::ty::{ParamEnvAnd, TyCtxt}; use syntax::codemap::DUMMY_SP; @@ -27,7 +27,7 @@ crate fn evaluate_obligation<'tcx>( _canonical_inference_vars, ) = infcx.instantiate_canonical_with_fresh_inference_vars(DUMMY_SP, &goal); - let mut selcx = SelectionContext::new(&infcx); + let mut selcx = SelectionContext::with_query_mode(&infcx, TraitQueryMode::Canonical); let obligation = Obligation::new(ObligationCause::dummy(), param_env, predicate); match selcx.evaluate_obligation_recursively(&obligation) { From 5cb0372160d4c97b967aa46e05ceb850be0cce44 Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Thu, 19 Apr 2018 02:49:21 -0500 Subject: [PATCH 6/8] Remove the stored obligation in OverflowError to simplify things We will shortly refactor things so that it is no longer needed --- src/librustc/traits/error_reporting.rs | 2 +- src/librustc/traits/mod.rs | 3 +- src/librustc/traits/select.rs | 39 +++++++++++----------- src/librustc/traits/structural_impls.rs | 2 +- src/librustc_traits/evaluate_obligation.rs | 4 +-- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 98684825d91f7..285d530a38a69 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -831,7 +831,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { err.struct_error(self.tcx, span, "constant expression") } - Overflow(_) => { + Overflow => { bug!("overflow should be handled before the `report_selection_error` path"); } }; diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 6a8a000b58a10..dd5208e908e19 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -362,8 +362,7 @@ pub enum SelectionError<'tcx> { ty::error::TypeError<'tcx>), TraitNotObjectSafe(DefId), ConstEvalFailure(ConstEvalErr<'tcx>), - // upon overflow, stores the obligation that hit the recursion limit - Overflow(TraitObligation<'tcx>), + Overflow, } pub struct FulfillmentError<'tcx> { diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 342b163b2f72b..fdf6dcf4bf37d 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -421,14 +421,13 @@ impl_stable_hash_for!(enum self::EvaluationResult { EvaluatedToErr }); -#[derive(Clone, Debug, PartialEq, Eq)] -/// Indicates that trait evaluation caused overflow. Stores the obligation -/// that hit the recursion limit. -pub struct OverflowError<'tcx>(pub TraitObligation<'tcx>); +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +/// Indicates that trait evaluation caused overflow. +pub struct OverflowError; -impl<'tcx> From> for SelectionError<'tcx> { - fn from(OverflowError(o): OverflowError<'tcx>) -> SelectionError<'tcx> { - SelectionError::Overflow(o) +impl<'tcx> From for SelectionError<'tcx> { + fn from(OverflowError: OverflowError) -> SelectionError<'tcx> { + SelectionError::Overflow } } @@ -573,7 +572,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { assert!(self.query_mode == TraitQueryMode::Standard); let candidate = match self.candidate_from_obligation(&stack) { - Err(SelectionError::Overflow(_)) => + Err(SelectionError::Overflow) => bug!("Overflow should be caught earlier in standard query mode"), Err(e) => { return Err(e); }, Ok(None) => { return Ok(None); }, @@ -581,7 +580,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { }; match self.confirm_candidate(obligation, candidate) { - Err(SelectionError::Overflow(_)) => + Err(SelectionError::Overflow) => bug!("Overflow should be caught earlier in standard query mode"), Err(e) => Err(e), Ok(candidate) => Ok(Some(candidate)) @@ -619,7 +618,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { /// an `EvaluationResult`. pub fn evaluate_obligation_recursively(&mut self, obligation: &PredicateObligation<'tcx>) - -> Result> + -> Result { self.probe(|this, _| { this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation) @@ -632,7 +631,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_predicates_recursively<'a,'o,I>(&mut self, stack: TraitObligationStackList<'o, 'tcx>, predicates: I) - -> Result> + -> Result where I : IntoIterator>, 'tcx:'a { let mut result = EvaluatedToOk; @@ -654,7 +653,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_predicate_recursively<'o>(&mut self, previous_stack: TraitObligationStackList<'o, 'tcx>, obligation: &PredicateObligation<'tcx>) - -> Result> + -> Result { debug!("evaluate_predicate_recursively({:?})", obligation); @@ -775,7 +774,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_trait_predicate_recursively<'o>(&mut self, previous_stack: TraitObligationStackList<'o, 'tcx>, mut obligation: TraitObligation<'tcx>) - -> Result> + -> Result { debug!("evaluate_trait_predicate_recursively({:?})", obligation); @@ -810,7 +809,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_stack<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) - -> Result> + -> Result { // In intercrate mode, whenever any of the types are unbound, // there can always be an impl. Even if there are no impls in @@ -921,7 +920,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { match self.candidate_from_obligation(stack) { Ok(Some(c)) => self.evaluate_candidate(stack, &c), Ok(None) => Ok(EvaluatedToAmbig), - Err(Overflow(o)) => Err(OverflowError(o)), + Err(Overflow) => Err(OverflowError), Err(..) => Ok(EvaluatedToErr) } } @@ -960,7 +959,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_candidate<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>, candidate: &SelectionCandidate<'tcx>) - -> Result> + -> Result { debug!("evaluate_candidate: depth={} candidate={:?}", stack.obligation.recursion_depth, candidate); @@ -1056,7 +1055,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { self.infcx().report_overflow_error(&stack.obligation, true); }, TraitQueryMode::Canonical => { - return Err(Overflow(stack.obligation.clone())); + return Err(Overflow); }, } } @@ -1144,7 +1143,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { .vec .iter() .map(|c| self.evaluate_candidate(stack, &c)) - .collect::, OverflowError<'_>>>()? + .collect::, OverflowError>>()? .iter() .all(|r| !r.may_apply()); if !candidate_set.ambiguous && no_candidates_apply { @@ -1224,7 +1223,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { evaluation: eval, })), Ok(_) => Ok(None), - Err(OverflowError(o)) => Err(Overflow(o)), + Err(OverflowError) => Err(Overflow), }) .collect(); @@ -1621,7 +1620,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { fn evaluate_where_clause<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>, where_clause_trait_ref: ty::PolyTraitRef<'tcx>) - -> Result> + -> Result { self.probe(move |this, _| { match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) { diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index c124cd7942a68..d7e42655bbb5b 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -177,7 +177,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::SelectionError<'a> { super::ConstEvalFailure(ref err) => { tcx.lift(err).map(super::ConstEvalFailure) } - super::Overflow(_) => bug!() // FIXME: ape ConstEvalFailure? + super::Overflow => bug!() // FIXME: ape ConstEvalFailure? } } } diff --git a/src/librustc_traits/evaluate_obligation.rs b/src/librustc_traits/evaluate_obligation.rs index a9cf35c4d70d4..f346bb8dc996b 100644 --- a/src/librustc_traits/evaluate_obligation.rs +++ b/src/librustc_traits/evaluate_obligation.rs @@ -32,8 +32,8 @@ crate fn evaluate_obligation<'tcx>( match selcx.evaluate_obligation_recursively(&obligation) { Ok(result) => result, - Err(OverflowError(o)) => { - infcx.report_overflow_error(&o, true) + Err(OverflowError) => { + infcx.report_overflow_error(&obligation, true) } } }) From d5b2e907444cb202407771eee9dd41c2518c1724 Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Thu, 19 Apr 2018 03:15:36 -0500 Subject: [PATCH 7/8] Retry canonical trait query in standard mode if overflow occurs This is slightly hacky and hopefully only a somewhat temporary solution. --- .../traits/query/evaluate_obligation.rs | 32 ++++++++++++++----- src/librustc/traits/select.rs | 19 +++++++---- src/librustc/ty/maps/mod.rs | 5 +-- src/librustc_traits/evaluate_obligation.rs | 9 ++---- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/librustc/traits/query/evaluate_obligation.rs b/src/librustc/traits/query/evaluate_obligation.rs index 88c51d006db06..4e028cac49abe 100644 --- a/src/librustc/traits/query/evaluate_obligation.rs +++ b/src/librustc/traits/query/evaluate_obligation.rs @@ -10,7 +10,8 @@ use infer::InferCtxt; use infer::canonical::{Canonical, Canonicalize}; -use traits::{EvaluationResult, PredicateObligation}; +use traits::{EvaluationResult, PredicateObligation, SelectionContext, + TraitQueryMode, OverflowError}; use traits::query::CanonicalPredicateGoal; use ty::{ParamEnvAnd, Predicate, TyCtxt}; @@ -21,10 +22,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, ) -> bool { - let (c_pred, _) = - self.canonicalize_query(&obligation.param_env.and(obligation.predicate)); - - self.tcx.global_tcx().evaluate_obligation(c_pred).may_apply() + self.evaluate_obligation(obligation).may_apply() } /// Evaluates whether the predicate can be satisfied in the given @@ -34,11 +32,29 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, ) -> bool { + self.evaluate_obligation(obligation) == EvaluationResult::EvaluatedToOk + } + + // Helper function that canonicalizes and runs the query, as well as handles + // overflow. + fn evaluate_obligation( + &self, + obligation: &PredicateObligation<'tcx>, + ) -> EvaluationResult { let (c_pred, _) = self.canonicalize_query(&obligation.param_env.and(obligation.predicate)); - - self.tcx.global_tcx().evaluate_obligation(c_pred) == - EvaluationResult::EvaluatedToOk + // Run canonical query. If overflow occurs, rerun from scratch but this time + // in standard trait query mode so that overflow is handled appropriately + // within `SelectionContext`. + match self.tcx.global_tcx().evaluate_obligation(c_pred) { + Ok(result) => result, + Err(OverflowError) => { + let mut selcx = + SelectionContext::with_query_mode(&self, TraitQueryMode::Standard); + selcx.evaluate_obligation_recursively(obligation) + .expect("Overflow should be caught earlier in standard query mode") + } + } } } diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index fdf6dcf4bf37d..4ba3655bb644a 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -425,6 +425,8 @@ impl_stable_hash_for!(enum self::EvaluationResult { /// Indicates that trait evaluation caused overflow. pub struct OverflowError; +impl_stable_hash_for!(struct OverflowError { }); + impl<'tcx> From for SelectionError<'tcx> { fn from(OverflowError: OverflowError) -> SelectionError<'tcx> { SelectionError::Overflow @@ -568,20 +570,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { let stack = self.push_stack(TraitObligationStackList::empty(), obligation); - // `select` is currently only called in standard query mode - assert!(self.query_mode == TraitQueryMode::Standard); - let candidate = match self.candidate_from_obligation(&stack) { - Err(SelectionError::Overflow) => - bug!("Overflow should be caught earlier in standard query mode"), + Err(SelectionError::Overflow) => { + // In standard mode, overflow must have been caught and reported + // earlier. + assert!(self.query_mode == TraitQueryMode::Canonical); + return Err(SelectionError::Overflow); + }, Err(e) => { return Err(e); }, Ok(None) => { return Ok(None); }, Ok(Some(candidate)) => candidate }; match self.confirm_candidate(obligation, candidate) { - Err(SelectionError::Overflow) => - bug!("Overflow should be caught earlier in standard query mode"), + Err(SelectionError::Overflow) => { + assert!(self.query_mode == TraitQueryMode::Canonical); + return Err(SelectionError::Overflow); + }, Err(e) => Err(e), Ok(candidate) => Ok(Some(candidate)) } diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs index 9343eccd38e9f..cb929225bcdcf 100644 --- a/src/librustc/ty/maps/mod.rs +++ b/src/librustc/ty/maps/mod.rs @@ -436,8 +436,9 @@ define_maps! { <'tcx> /// Do not call this query directly: invoke `infcx.predicate_may_hold()` or /// `infcx.predicate_must_hold()` instead. - [] fn evaluate_obligation: - EvaluateObligation(CanonicalPredicateGoal<'tcx>) -> traits::EvaluationResult, + [] fn evaluate_obligation: EvaluateObligation( + CanonicalPredicateGoal<'tcx> + ) -> Result, [] fn substitute_normalize_and_test_predicates: substitute_normalize_and_test_predicates_node((DefId, &'tcx Substs<'tcx>)) -> bool, diff --git a/src/librustc_traits/evaluate_obligation.rs b/src/librustc_traits/evaluate_obligation.rs index f346bb8dc996b..21259bbcd38ff 100644 --- a/src/librustc_traits/evaluate_obligation.rs +++ b/src/librustc_traits/evaluate_obligation.rs @@ -17,7 +17,7 @@ use syntax::codemap::DUMMY_SP; crate fn evaluate_obligation<'tcx>( tcx: TyCtxt<'_, 'tcx, 'tcx>, goal: CanonicalPredicateGoal<'tcx>, -) -> EvaluationResult { +) -> Result { tcx.infer_ctxt().enter(|ref infcx| { let ( ParamEnvAnd { @@ -30,11 +30,6 @@ crate fn evaluate_obligation<'tcx>( let mut selcx = SelectionContext::with_query_mode(&infcx, TraitQueryMode::Canonical); let obligation = Obligation::new(ObligationCause::dummy(), param_env, predicate); - match selcx.evaluate_obligation_recursively(&obligation) { - Ok(result) => result, - Err(OverflowError) => { - infcx.report_overflow_error(&obligation, true) - } - } + selcx.evaluate_obligation_recursively(&obligation) }) } From e423dcc7133f20c8da2609dba96afeeab1efbc5a Mon Sep 17 00:00:00 2001 From: Aravind Gollakota Date: Sun, 22 Apr 2018 16:48:38 -0500 Subject: [PATCH 8/8] Update a compile-fail test --- src/test/compile-fail/issue-23080-2.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/compile-fail/issue-23080-2.rs b/src/test/compile-fail/issue-23080-2.rs index 2aa87f8424b95..fc365a4b9aaee 100644 --- a/src/test/compile-fail/issue-23080-2.rs +++ b/src/test/compile-fail/issue-23080-2.rs @@ -10,6 +10,8 @@ // ignore-tidy-linelength +//~^^^^^^^^^^^^ ERROR + #![feature(optin_builtin_traits)] unsafe auto trait Trait { @@ -22,5 +24,4 @@ fn call_method(x: T) {} fn main() { // ICE call_method(()); - //~^ ERROR }