From 1548e0fb01a76360d2786da4a760cc82a55f6ed7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 17 Jan 2021 21:02:12 -0500 Subject: [PATCH 1/2] Inline `evaluate_obligation` instead of going through the query system There are several reasons I changed this: - `evaluate_obligation` is only used in one place in `rustc_trait_selection`, and in fact warns you against using it anywhere else. - The implementation in `rustc_traits` was considerably more complicated than necessary, because it didn't have the context available in `rustc_trait_selection`. - It allows moving OverflowError into rustc_trait_selection, making https://github.com/rust-lang/rust/issues/81091 simpler (in particular, it allows holding an `Obligation` in OverflowError, which is only defined in `rustc_infer` and not available in rustc_middle). The only reason to keep the previous behavior is if the cache from the query system made it significantly faster. --- compiler/rustc_middle/src/query/mod.rs | 14 ++------ compiler/rustc_middle/src/ty/query/mod.rs | 6 ++-- .../src/traits/query/evaluate_obligation.rs | 10 ++---- .../rustc_traits/src/evaluate_obligation.rs | 32 ------------------- compiler/rustc_traits/src/lib.rs | 2 -- 5 files changed, 8 insertions(+), 56 deletions(-) delete mode 100644 compiler/rustc_traits/src/evaluate_obligation.rs diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 00ee7b8ec7709..b371d9ccb24e6 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2,9 +2,9 @@ use crate::dep_graph::SerializedDepNodeIndex; use crate::mir::interpret::{GlobalId, LitToConstInput}; use crate::traits; use crate::traits::query::{ - CanonicalPredicateGoal, CanonicalProjectionGoal, CanonicalTyGoal, - CanonicalTypeOpAscribeUserTypeGoal, CanonicalTypeOpEqGoal, CanonicalTypeOpNormalizeGoal, - CanonicalTypeOpProvePredicateGoal, CanonicalTypeOpSubtypeGoal, + CanonicalProjectionGoal, CanonicalTyGoal, CanonicalTypeOpAscribeUserTypeGoal, + CanonicalTypeOpEqGoal, CanonicalTypeOpNormalizeGoal, CanonicalTypeOpProvePredicateGoal, + CanonicalTypeOpSubtypeGoal, }; use crate::ty::query::queries; use crate::ty::subst::{GenericArg, SubstsRef}; @@ -1531,14 +1531,6 @@ rustc_queries! { desc { "computing dropck types for `{:?}`", goal } } - /// Do not call this query directly: invoke `infcx.predicate_may_hold()` or - /// `infcx.predicate_must_hold()` instead. - query evaluate_obligation( - goal: CanonicalPredicateGoal<'tcx> - ) -> Result { - desc { "evaluating trait selection obligation `{}`", goal.value.value } - } - query evaluate_goal( goal: traits::CanonicalChalkEnvironmentAndGoal<'tcx> ) -> Result< diff --git a/compiler/rustc_middle/src/ty/query/mod.rs b/compiler/rustc_middle/src/ty/query/mod.rs index acfa58e511ed1..089b77280b1cf 100644 --- a/compiler/rustc_middle/src/ty/query/mod.rs +++ b/compiler/rustc_middle/src/ty/query/mod.rs @@ -18,9 +18,9 @@ use crate::mir::interpret::{ConstValue, EvalToAllocationRawResult, EvalToConstVa use crate::mir::interpret::{LitToConstError, LitToConstInput}; use crate::mir::mono::CodegenUnit; use crate::traits::query::{ - CanonicalPredicateGoal, CanonicalProjectionGoal, CanonicalTyGoal, - CanonicalTypeOpAscribeUserTypeGoal, CanonicalTypeOpEqGoal, CanonicalTypeOpNormalizeGoal, - CanonicalTypeOpProvePredicateGoal, CanonicalTypeOpSubtypeGoal, NoSolution, + CanonicalProjectionGoal, CanonicalTyGoal, CanonicalTypeOpAscribeUserTypeGoal, + CanonicalTypeOpEqGoal, CanonicalTypeOpNormalizeGoal, CanonicalTypeOpProvePredicateGoal, + CanonicalTypeOpSubtypeGoal, NoSolution, }; use crate::traits::query::{ DropckOutlivesResult, DtorckConstraint, MethodAutoderefStepsResult, NormalizationResult, diff --git a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs index b83a4cd1e5775..e8380ccfd5a95 100644 --- a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs +++ b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs @@ -1,4 +1,3 @@ -use crate::infer::canonical::OriginalQueryValues; use crate::infer::InferCtxt; use crate::traits::{ EvaluationResult, OverflowError, PredicateObligation, SelectionContext, TraitQueryMode, @@ -63,13 +62,8 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, ) -> Result { - let mut _orig_values = OriginalQueryValues::default(); - let c_pred = self - .canonicalize_query(obligation.param_env.and(obligation.predicate), &mut _orig_values); - // 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`. - self.tcx.evaluate_obligation(c_pred) + let mut selcx = SelectionContext::with_query_mode(self, TraitQueryMode::Canonical); + selcx.evaluate_root_obligation(&obligation) } // Helper function that canonicalizes and runs the query. If an diff --git a/compiler/rustc_traits/src/evaluate_obligation.rs b/compiler/rustc_traits/src/evaluate_obligation.rs deleted file mode 100644 index 2404b7ff4b54a..0000000000000 --- a/compiler/rustc_traits/src/evaluate_obligation.rs +++ /dev/null @@ -1,32 +0,0 @@ -use rustc_infer::infer::TyCtxtInferExt; -use rustc_middle::ty::query::Providers; -use rustc_middle::ty::{ParamEnvAnd, TyCtxt}; -use rustc_span::source_map::DUMMY_SP; -use rustc_trait_selection::traits::query::CanonicalPredicateGoal; -use rustc_trait_selection::traits::{ - EvaluationResult, Obligation, ObligationCause, OverflowError, SelectionContext, TraitQueryMode, -}; - -crate fn provide(p: &mut Providers) { - *p = Providers { evaluate_obligation, ..*p }; -} - -fn evaluate_obligation<'tcx>( - tcx: TyCtxt<'tcx>, - canonical_goal: CanonicalPredicateGoal<'tcx>, -) -> Result { - debug!("evaluate_obligation(canonical_goal={:#?})", canonical_goal); - tcx.infer_ctxt().enter_with_canonical( - DUMMY_SP, - &canonical_goal, - |ref infcx, goal, _canonical_inference_vars| { - debug!("evaluate_obligation: goal={:#?}", goal); - let ParamEnvAnd { param_env, value: predicate } = goal; - - let mut selcx = SelectionContext::with_query_mode(&infcx, TraitQueryMode::Canonical); - let obligation = Obligation::new(ObligationCause::dummy(), param_env, predicate); - - selcx.evaluate_root_obligation(&obligation) - }, - ) -} diff --git a/compiler/rustc_traits/src/lib.rs b/compiler/rustc_traits/src/lib.rs index 7b688cd3e2199..94377389cbda8 100644 --- a/compiler/rustc_traits/src/lib.rs +++ b/compiler/rustc_traits/src/lib.rs @@ -14,7 +14,6 @@ extern crate rustc_middle; mod chalk; mod dropck_outlives; -mod evaluate_obligation; mod implied_outlives_bounds; mod normalize_erasing_regions; mod normalize_projection_ty; @@ -24,7 +23,6 @@ use rustc_middle::ty::query::Providers; pub fn provide(p: &mut Providers) { dropck_outlives::provide(p); - evaluate_obligation::provide(p); implied_outlives_bounds::provide(p); chalk::provide(p); normalize_projection_ty::provide(p); From 81ea1b93e1fdd390ef75be7c21028045bc3d58b5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 17 Jan 2021 21:44:07 -0500 Subject: [PATCH 2/2] Add back canonicalization --- .../src/traits/query/evaluate_obligation.rs | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs index e8380ccfd5a95..0c753be158f08 100644 --- a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs +++ b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs @@ -1,6 +1,11 @@ +use rustc_infer::infer::TyCtxtInferExt; +use rustc_span::DUMMY_SP; + use crate::infer::InferCtxt; +use crate::infer::canonical::OriginalQueryValues; +use crate::traits::ty::ParamEnvAnd; use crate::traits::{ - EvaluationResult, OverflowError, PredicateObligation, SelectionContext, TraitQueryMode, + EvaluationResult, Obligation, ObligationCause, OverflowError, PredicateObligation, SelectionContext, TraitQueryMode, }; pub trait InferCtxtExt<'tcx> { @@ -62,8 +67,24 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, ) -> Result { - let mut selcx = SelectionContext::with_query_mode(self, TraitQueryMode::Canonical); - selcx.evaluate_root_obligation(&obligation) + let mut _orig_values = OriginalQueryValues::default(); + let c_pred = self + .canonicalize_query(obligation.param_env.and(obligation.predicate), &mut _orig_values); + + debug!("evaluate_obligation: c_pred={:#?}", c_pred); + self.tcx.infer_ctxt().enter_with_canonical( + DUMMY_SP, + &c_pred, + |ref infcx, goal, _canonical_inference_vars| { + debug!("evaluate_obligation: goal={:#?}", goal); + let ParamEnvAnd { param_env, value: predicate } = goal; + + let mut selcx = SelectionContext::with_query_mode(&infcx, TraitQueryMode::Canonical); + let obligation = Obligation::new(ObligationCause::dummy(), param_env, predicate); + + selcx.evaluate_root_obligation(&obligation) + }, + ) } // Helper function that canonicalizes and runs the query. If an