about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-07-21 23:18:19 +0000
committerbors <bors@rust-lang.org>2021-07-21 23:18:19 +0000
commit602150f21fd8a9bd3e903039c0d8c008d85aa7f1 (patch)
tree0c56ed01b7dc446e5b765c839ed301fd71719665
parent32c9b7b091534f6d80e7e85da0cd425acb6c9a79 (diff)
parent3291218f476ab5e7d4603212cff91f8435046046 (diff)
downloadrust-602150f21fd8a9bd3e903039c0d8c008d85aa7f1.tar.gz
rust-602150f21fd8a9bd3e903039c0d8c008d85aa7f1.zip
Auto merge of #86946 - Aaron1011:eval-proj-obligation, r=nikomatsakis
 Improve caching during trait evaluation

Previously, we would 'forget' that we had `'static` regions in some
place during trait evaluation. This lead to us producing
`EvaluatedToOkModuloRegions` when we could have produced
`EvaluatedToOk`, causing us to perform unnecessary work.

This PR preserves `'static` regions when we canonicalize a predicate for
`evaluate_obligation`, and when we 'freshen' a predicate during trait
evaluation. Thie ensures that evaluating a predicate containing
`'static` regions can produce `EvaluatedToOk` (assuming that we
don't end up introducing any region dependencies during evaluation).

Building off of this improved caching, we use
`predicate_must_hold_considering_regions` during fulfillment of
projection predicates to see if we can skip performing additional work.
We already do this for trait predicates, but doing this for projection
predicates lead to mixed performance results without the above caching
improvements.
-rw-r--r--compiler/rustc_infer/src/infer/canonical/canonicalizer.rs19
-rw-r--r--compiler/rustc_infer/src/infer/freshen.rs14
-rw-r--r--compiler/rustc_infer/src/infer/mod.rs7
-rw-r--r--compiler/rustc_trait_selection/src/traits/fulfill.rs14
-rw-r--r--compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs6
-rw-r--r--compiler/rustc_trait_selection/src/traits/query/normalize.rs4
-rw-r--r--compiler/rustc_trait_selection/src/traits/query/type_op/mod.rs5
-rw-r--r--compiler/rustc_trait_selection/src/traits/select/mod.rs8
8 files changed, 49 insertions, 28 deletions
diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
index ac953f4305c..448dd662348 100644
--- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
+++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
@@ -102,20 +102,11 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
         )
     }
 
-    /// A hacky variant of `canonicalize_query` that does not
-    /// canonicalize `'static`. Unfortunately, the existing leak
-    /// check treats `'static` differently in some cases (see also
-    /// #33684), so if we are performing an operation that may need to
-    /// prove "leak-check" related things, we leave `'static`
-    /// alone.
-    ///
-    /// `'static` is also special cased when winnowing candidates when
-    /// selecting implementation candidates, so we also have to leave `'static`
-    /// alone for queries that do selection.
-    //
-    // FIXME(#48536): once the above issues are resolved, we can remove this
-    // and just use `canonicalize_query`.
-    pub fn canonicalize_hr_query_hack<V>(
+    /// A variant of `canonicalize_query` that does not
+    /// canonicalize `'static`. This is useful when
+    /// the query implementation can perform more efficient
+    /// handling of `'static` regions (e.g. trait evaluation).
+    pub fn canonicalize_query_keep_static<V>(
         &self,
         value: V,
         query_state: &mut OriginalQueryValues<'tcx>,
diff --git a/compiler/rustc_infer/src/infer/freshen.rs b/compiler/rustc_infer/src/infer/freshen.rs
index b3d7876c6e8..4af1bdf97a7 100644
--- a/compiler/rustc_infer/src/infer/freshen.rs
+++ b/compiler/rustc_infer/src/infer/freshen.rs
@@ -47,16 +47,18 @@ pub struct TypeFreshener<'a, 'tcx> {
     const_freshen_count: u32,
     ty_freshen_map: FxHashMap<ty::InferTy, Ty<'tcx>>,
     const_freshen_map: FxHashMap<ty::InferConst<'tcx>, &'tcx ty::Const<'tcx>>,
+    keep_static: bool,
 }
 
 impl<'a, 'tcx> TypeFreshener<'a, 'tcx> {
-    pub fn new(infcx: &'a InferCtxt<'a, 'tcx>) -> TypeFreshener<'a, 'tcx> {
+    pub fn new(infcx: &'a InferCtxt<'a, 'tcx>, keep_static: bool) -> TypeFreshener<'a, 'tcx> {
         TypeFreshener {
             infcx,
             ty_freshen_count: 0,
             const_freshen_count: 0,
             ty_freshen_map: Default::default(),
             const_freshen_map: Default::default(),
+            keep_static,
         }
     }
 
@@ -124,8 +126,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for TypeFreshener<'a, 'tcx> {
                 r
             }
 
-            ty::ReStatic
-            | ty::ReEarlyBound(..)
+            ty::ReEarlyBound(..)
             | ty::ReFree(_)
             | ty::ReVar(_)
             | ty::RePlaceholder(..)
@@ -134,6 +135,13 @@ impl<'a, 'tcx> TypeFolder<'tcx> for TypeFreshener<'a, 'tcx> {
                 // replace all free regions with 'erased
                 self.tcx().lifetimes.re_erased
             }
+            ty::ReStatic => {
+                if self.keep_static {
+                    r
+                } else {
+                    self.tcx().lifetimes.re_erased
+                }
+            }
         }
     }
 
diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs
index f39431f2494..d3bfb2b2e44 100644
--- a/compiler/rustc_infer/src/infer/mod.rs
+++ b/compiler/rustc_infer/src/infer/mod.rs
@@ -646,7 +646,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     }
 
     pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> {
-        freshen::TypeFreshener::new(self)
+        freshen::TypeFreshener::new(self, false)
+    }
+
+    /// Like `freshener`, but does not replace `'static` regions.
+    pub fn freshener_keep_static<'b>(&'b self) -> TypeFreshener<'b, 'tcx> {
+        freshen::TypeFreshener::new(self, true)
     }
 
     pub fn type_is_unconstrained_numeric(&'a self, ty: Ty<'_>) -> UnconstrainedNumeric {
diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs
index dfe2909498d..9ec1dd5c2ee 100644
--- a/compiler/rustc_trait_selection/src/traits/fulfill.rs
+++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs
@@ -365,6 +365,7 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
                     let project_obligation = obligation.with(binder.rebind(data));
 
                     self.process_projection_obligation(
+                        obligation,
                         project_obligation,
                         &mut pending_obligation.stalled_on,
                     )
@@ -419,6 +420,7 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
                     let project_obligation = obligation.with(Binder::dummy(*data));
 
                     self.process_projection_obligation(
+                        obligation,
                         project_obligation,
                         &mut pending_obligation.stalled_on,
                     )
@@ -666,10 +668,22 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
 
     fn process_projection_obligation(
         &mut self,
+        obligation: &PredicateObligation<'tcx>,
         project_obligation: PolyProjectionObligation<'tcx>,
         stalled_on: &mut Vec<TyOrConstInferVar<'tcx>>,
     ) -> ProcessResult<PendingPredicateObligation<'tcx>, FulfillmentErrorCode<'tcx>> {
         let tcx = self.selcx.tcx();
+
+        if obligation.predicate.is_global() {
+            // no type variables present, can use evaluation for better caching.
+            // FIXME: consider caching errors too.
+            if self.selcx.infcx().predicate_must_hold_considering_regions(obligation) {
+                return ProcessResult::Changed(vec![]);
+            } else {
+                tracing::debug!("Does NOT hold: {:?}", obligation);
+            }
+        }
+
         match project::poly_project_and_unify_type(self.selcx, &project_obligation) {
             Ok(Ok(Some(os))) => ProcessResult::Changed(mk_pending(os)),
             Ok(Ok(None)) => {
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 b83a4cd1e57..2dc48e47efc 100644
--- a/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs
+++ b/compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs
@@ -64,8 +64,10 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
         let mut _orig_values = OriginalQueryValues::default();
-        let c_pred = self
-            .canonicalize_query(obligation.param_env.and(obligation.predicate), &mut _orig_values);
+        let c_pred = self.canonicalize_query_keep_static(
+            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`.
diff --git a/compiler/rustc_trait_selection/src/traits/query/normalize.rs b/compiler/rustc_trait_selection/src/traits/query/normalize.rs
index d65a378b1ed..3f6efa03b3a 100644
--- a/compiler/rustc_trait_selection/src/traits/query/normalize.rs
+++ b/compiler/rustc_trait_selection/src/traits/query/normalize.rs
@@ -180,7 +180,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> {
                 // so we cannot canonicalize it.
                 let c_data = self
                     .infcx
-                    .canonicalize_hr_query_hack(self.param_env.and(data), &mut orig_values);
+                    .canonicalize_query_keep_static(self.param_env.and(data), &mut orig_values);
                 debug!("QueryNormalizer: c_data = {:#?}", c_data);
                 debug!("QueryNormalizer: orig_values = {:#?}", orig_values);
                 match tcx.normalize_projection_ty(c_data) {
@@ -249,7 +249,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> {
                 // so we cannot canonicalize it.
                 let c_data = self
                     .infcx
-                    .canonicalize_hr_query_hack(self.param_env.and(data), &mut orig_values);
+                    .canonicalize_query_keep_static(self.param_env.and(data), &mut orig_values);
                 debug!("QueryNormalizer: c_data = {:#?}", c_data);
                 debug!("QueryNormalizer: orig_values = {:#?}", orig_values);
                 let normalized_ty = match tcx.normalize_projection_ty(c_data) {
diff --git a/compiler/rustc_trait_selection/src/traits/query/type_op/mod.rs b/compiler/rustc_trait_selection/src/traits/query/type_op/mod.rs
index 130ffa1a33a..fbff86618ad 100644
--- a/compiler/rustc_trait_selection/src/traits/query/type_op/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/query/type_op/mod.rs
@@ -77,12 +77,13 @@ pub trait QueryTypeOp<'tcx>: fmt::Debug + Sized + TypeFoldable<'tcx> + 'tcx {
         }
 
         // FIXME(#33684) -- We need to use
-        // `canonicalize_hr_query_hack` here because of things
+        // `canonicalize_query_keep_static` here because of things
         // like the subtype query, which go awry around
         // `'static` otherwise.
         let mut canonical_var_values = OriginalQueryValues::default();
         let old_param_env = query_key.param_env;
-        let canonical_self = infcx.canonicalize_hr_query_hack(query_key, &mut canonical_var_values);
+        let canonical_self =
+            infcx.canonicalize_query_keep_static(query_key, &mut canonical_var_values);
         let canonical_result = Self::perform_query(infcx.tcx, canonical_self)?;
 
         let InferOk { value, obligations } = infcx
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index 1bdc8b34cf6..95611ebc818 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -216,7 +216,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     pub fn new(infcx: &'cx InferCtxt<'cx, 'tcx>) -> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
-            freshener: infcx.freshener(),
+            freshener: infcx.freshener_keep_static(),
             intercrate: false,
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
@@ -227,7 +227,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'tcx>) -> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
-            freshener: infcx.freshener(),
+            freshener: infcx.freshener_keep_static(),
             intercrate: true,
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
@@ -242,7 +242,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         debug!(?allow_negative_impls, "with_negative");
         SelectionContext {
             infcx,
-            freshener: infcx.freshener(),
+            freshener: infcx.freshener_keep_static(),
             intercrate: false,
             intercrate_ambiguity_causes: None,
             allow_negative_impls,
@@ -257,7 +257,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         debug!(?query_mode, "with_query_mode");
         SelectionContext {
             infcx,
-            freshener: infcx.freshener(),
+            freshener: infcx.freshener_keep_static(),
             intercrate: false,
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,