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.

Author: bors

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>,

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
+                }
+            }
         }
     }
 

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 {

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)) => {

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`.

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) {

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

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,