Revert "Auto merge of rust-lang#107376 - aliemjay:remove-givens, r=lcnr"

This reverts commit e84e5ff, reversing
changes made to 1716932.

Author: lqd

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -330,6 +330,7 @@ fn compare_method_predicate_entailment<'tcx>(
     // lifetime parameters.
     let outlives_env = OutlivesEnvironment::with_bounds(
         param_env,
+        Some(infcx),
         infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys.clone()),
     );
     infcx.process_registered_region_obligations(
@@ -727,6 +728,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
     // lifetime parameters.
     let outlives_environment = OutlivesEnvironment::with_bounds(
         param_env,
+        Some(infcx),
         infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys),
     );
     infcx
@@ -2056,7 +2058,8 @@ pub(super) fn check_type_bounds<'tcx>(
     // Finally, resolve all regions. This catches wily misuses of
     // lifetime parameters.
     let implied_bounds = infcx.implied_bounds_tys(param_env, impl_ty_def_id, assumed_wf_types);
-    let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
+    let outlives_environment =
+        OutlivesEnvironment::with_bounds(param_env, Some(&infcx), implied_bounds);
 
     infcx.err_ctxt().check_region_obligations_and_report_errors(
         impl_ty.def_id.expect_local(),

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -115,7 +115,8 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
         return;
     }
 
-    let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
+    let outlives_environment =
+        OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds);
 
     let _ = infcx
         .err_ctxt()
@@ -675,6 +676,7 @@ fn resolve_regions_with_wf_tys<'tcx>(
     let infcx = tcx.infer_ctxt().build();
     let outlives_environment = OutlivesEnvironment::with_bounds(
         param_env,
+        Some(&infcx),
         infcx.implied_bounds_tys(param_env, id, wf_tys.clone()),
     );
     let region_bound_pairs = outlives_environment.region_bound_pairs();

compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs

@@ -179,7 +179,7 @@ fn get_impl_substs(
     }
 
     let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_def_id, assumed_wf_types);
-    let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
+    let outlives_env = OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds);
     let _ =
         infcx.err_ctxt().check_region_obligations_and_report_errors(impl1_def_id, &outlives_env);
     let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {

compiler/rustc_hir_typeck/src/pat.rs

@@ -238,8 +238,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // Note that there are two tests to check that this remains true
         // (`regions-reassign-{match,let}-bound-pointer.rs`).
         //
-        // 2. An outdated issue related to the old HIR borrowck. See the test
+        // 2. Things go horribly wrong if we use subtype. The reason for
+        // THIS is a fairly subtle case involving bound regions. See the
+        // `givens` field in `region_constraints`, as well as the test
         // `regions-relate-bound-regions-on-closures-to-inference-variables.rs`,
+        // for details. Short version is that we must sometimes detect
+        // relationships between specific region variables and regions
+        // bound in a closure signature, and that detection gets thrown
+        // off when we substitute fresh region variables here to enable
+        // subtyping.
     }
 
     /// Compute the new expected type and default binding mode from the old ones

compiler/rustc_infer/src/infer/canonical/query_response.rs

@@ -636,9 +636,11 @@ pub fn make_query_region_constraints<'tcx>(
     outlives_obligations: impl Iterator<Item = (Ty<'tcx>, ty::Region<'tcx>, ConstraintCategory<'tcx>)>,
     region_constraints: &RegionConstraintData<'tcx>,
 ) -> QueryRegionConstraints<'tcx> {
-    let RegionConstraintData { constraints, verifys, member_constraints } = region_constraints;
+    let RegionConstraintData { constraints, verifys, givens, member_constraints } =
+        region_constraints;
 
     assert!(verifys.is_empty());
+    assert!(givens.is_empty());
 
     debug!(?constraints);
 

compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs

@@ -13,7 +13,7 @@ use rustc_data_structures::graph::implementation::{
     Direction, Graph, NodeIndex, INCOMING, OUTGOING,
 };
 use rustc_data_structures::intern::Interned;
-use rustc_index::vec::IndexVec;
+use rustc_index::vec::{Idx, IndexVec};
 use rustc_middle::ty::fold::TypeFoldable;
 use rustc_middle::ty::PlaceholderRegion;
 use rustc_middle::ty::{self, Ty, TyCtxt};
@@ -132,6 +132,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
         }
 
         let graph = self.construct_graph();
+        self.expand_givens(&graph);
         self.expansion(&mut var_data);
         self.collect_errors(&mut var_data, errors);
         self.collect_var_errors(&var_data, &graph, errors);
@@ -163,6 +164,38 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
         }
     }
 
+    fn expand_givens(&mut self, graph: &RegionGraph<'_>) {
+        // Givens are a kind of horrible hack to account for
+        // constraints like 'c <= '0 that are known to hold due to
+        // closure signatures (see the comment above on the `givens`
+        // field). They should go away. But until they do, the role
+        // of this fn is to account for the transitive nature:
+        //
+        //     Given 'c <= '0
+        //     and   '0 <= '1
+        //     then  'c <= '1
+
+        let seeds: Vec<_> = self.data.givens.iter().cloned().collect();
+        for (r, vid) in seeds {
+            // While all things transitively reachable in the graph
+            // from the variable (`'0` in the example above).
+            let seed_index = NodeIndex(vid.index() as usize);
+            for succ_index in graph.depth_traverse(seed_index, OUTGOING) {
+                let succ_index = succ_index.0;
+
+                // The first N nodes correspond to the region
+                // variables. Other nodes correspond to constant
+                // regions.
+                if succ_index < self.num_vars() {
+                    let succ_vid = RegionVid::new(succ_index);
+
+                    // Add `'c <= '1`.
+                    self.data.givens.insert((r, succ_vid));
+                }
+            }
+        }
+    }
+
     /// Gets the LUb of a given region and the empty region
     fn lub_empty(&self, a_region: Region<'tcx>) -> Result<Region<'tcx>, PlaceholderRegion> {
         match *a_region {
@@ -329,6 +362,18 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
     ) -> bool {
         debug!("expand_node({:?}, {:?} == {:?})", a_region, b_vid, b_data);
 
+        match *a_region {
+            // Check if this relationship is implied by a given.
+            ty::ReEarlyBound(_) | ty::ReFree(_) => {
+                if self.data.givens.contains(&(a_region, b_vid)) {
+                    debug!("given");
+                    return false;
+                }
+            }
+
+            _ => {}
+        }
+
         match *b_data {
             VarValue::Empty(empty_ui) => {
                 let lub = match self.lub_empty(a_region) {

compiler/rustc_infer/src/infer/mod.rs

@@ -855,6 +855,10 @@ impl<'tcx> InferCtxt<'tcx> {
         self.inner.borrow().undo_log.opaque_types_in_snapshot(&snapshot.undo_snapshot)
     }
 
+    pub fn add_given(&self, sub: ty::Region<'tcx>, sup: ty::RegionVid) {
+        self.inner.borrow_mut().unwrap_region_constraints().add_given(sub, sup);
+    }
+
     pub fn can_sub<T>(&self, param_env: ty::ParamEnv<'tcx>, a: T, b: T) -> bool
     where
         T: at::ToTrace<'tcx>,

compiler/rustc_infer/src/infer/outlives/env.rs

@@ -1,9 +1,9 @@
 use crate::infer::free_regions::FreeRegionMap;
-use crate::infer::GenericKind;
+use crate::infer::{GenericKind, InferCtxt};
 use crate::traits::query::OutlivesBound;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_data_structures::transitive_relation::TransitiveRelationBuilder;
-use rustc_middle::ty::{self, Region};
+use rustc_middle::ty::{self, ReEarlyBound, ReFree, ReVar, Region};
 
 use super::explicit_outlives_bounds;
 
@@ -75,7 +75,7 @@ impl<'tcx> OutlivesEnvironment<'tcx> {
             region_bound_pairs: Default::default(),
         };
 
-        builder.add_outlives_bounds(explicit_outlives_bounds(param_env));
+        builder.add_outlives_bounds(None, explicit_outlives_bounds(param_env));
 
         builder
     }
@@ -89,10 +89,11 @@ impl<'tcx> OutlivesEnvironment<'tcx> {
     /// Create a new `OutlivesEnvironment` with extra outlives bounds.
     pub fn with_bounds(
         param_env: ty::ParamEnv<'tcx>,
+        infcx: Option<&InferCtxt<'tcx>>,
         extra_bounds: impl IntoIterator<Item = OutlivesBound<'tcx>>,
     ) -> Self {
         let mut builder = Self::builder(param_env);
-        builder.add_outlives_bounds(extra_bounds);
+        builder.add_outlives_bounds(infcx, extra_bounds);
         builder.build()
     }
 
@@ -119,7 +120,12 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
     }
 
     /// Processes outlives bounds that are known to hold, whether from implied or other sources.
-    fn add_outlives_bounds<I>(&mut self, outlives_bounds: I)
+    ///
+    /// The `infcx` parameter is optional; if the implied bounds may
+    /// contain inference variables, it must be supplied, in which
+    /// case we will register "givens" on the inference context. (See
+    /// `RegionConstraintData`.)
+    fn add_outlives_bounds<I>(&mut self, infcx: Option<&InferCtxt<'tcx>>, outlives_bounds: I)
     where
         I: IntoIterator<Item = OutlivesBound<'tcx>>,
     {
@@ -136,14 +142,27 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
                     self.region_bound_pairs
                         .insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a));
                 }
-                OutlivesBound::RegionSubRegion(r_a, r_b) => match (*r_a, *r_b) {
-                    (
-                        ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_),
-                        ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_),
-                    ) => self.region_relation.add(r_a, r_b),
-                    (ty::ReError(_), _) | (_, ty::ReError(_)) => {}
-                    _ => bug!("add_outlives_bounds: unexpected regions"),
-                },
+                OutlivesBound::RegionSubRegion(r_a, r_b) => {
+                    if let (ReEarlyBound(_) | ReFree(_), ReVar(vid_b)) = (r_a.kind(), r_b.kind()) {
+                        infcx
+                            .expect("no infcx provided but region vars found")
+                            .add_given(r_a, vid_b);
+                    } else {
+                        // In principle, we could record (and take
+                        // advantage of) every relationship here, but
+                        // we are also free not to -- it simply means
+                        // strictly less that we can successfully type
+                        // check. Right now we only look for things
+                        // relationships between free regions. (It may
+                        // also be that we should revise our inference
+                        // system to be more general and to make use
+                        // of *every* relationship that arises here,
+                        // but presently we do not.)
+                        if r_a.is_free_or_static() && r_b.is_free() {
+                            self.region_relation.add(r_a, r_b)
+                        }
+                    }
+                }
             }
         }
     }

compiler/rustc_infer/src/infer/region_constraints/leak_check.rs

@@ -424,6 +424,9 @@ impl<'tcx> MiniGraph<'tcx> {
                 &AddConstraint(Constraint::RegSubReg(a, b)) => {
                     each_edge(a, b);
                 }
+                &AddGiven(a, b) => {
+                    each_edge(a, tcx.mk_re_var(b));
+                }
                 &AddVerify(i) => span_bug!(
                     verifys[i].origin.span(),
                     "we never add verifications while doing higher-ranked things",

compiler/rustc_infer/src/infer/region_constraints/mod.rs

@@ -7,7 +7,7 @@ use super::{
     InferCtxtUndoLogs, MiscVariable, RegionVariableOrigin, Rollback, Snapshot, SubregionOrigin,
 };
 
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
 use rustc_data_structures::intern::Interned;
 use rustc_data_structures::sync::Lrc;
 use rustc_data_structures::undo_log::UndoLogs;
@@ -104,6 +104,26 @@ pub struct RegionConstraintData<'tcx> {
     /// An example is a `A <= B` where neither `A` nor `B` are
     /// inference variables.
     pub verifys: Vec<Verify<'tcx>>,
+
+    /// A "given" is a relationship that is known to hold. In
+    /// particular, we often know from closure fn signatures that a
+    /// particular free region must be a subregion of a region
+    /// variable:
+    ///
+    ///    foo.iter().filter(<'a> |x: &'a &'b T| ...)
+    ///
+    /// In situations like this, `'b` is in fact a region variable
+    /// introduced by the call to `iter()`, and `'a` is a bound region
+    /// on the closure (as indicated by the `<'a>` prefix). If we are
+    /// naive, we wind up inferring that `'b` must be `'static`,
+    /// because we require that it be greater than `'a` and we do not
+    /// know what `'a` is precisely.
+    ///
+    /// This hashmap is used to avoid that naive scenario. Basically
+    /// we record the fact that `'a <= 'b` is implied by the fn
+    /// signature, and then ignore the constraint when solving
+    /// equations. This is a bit of a hack but seems to work.
+    pub givens: FxIndexSet<(Region<'tcx>, ty::RegionVid)>,
 }
 
 /// Represents a constraint that influences the inference process.
@@ -277,6 +297,9 @@ pub(crate) enum UndoLog<'tcx> {
     /// We added the given `verify`.
     AddVerify(usize),
 
+    /// We added the given `given`.
+    AddGiven(Region<'tcx>, ty::RegionVid),
+
     /// We added a GLB/LUB "combination variable".
     AddCombination(CombineMapType, TwoRegions<'tcx>),
 }
@@ -325,6 +348,9 @@ impl<'tcx> RegionConstraintStorage<'tcx> {
                 self.data.verifys.pop();
                 assert_eq!(self.data.verifys.len(), index);
             }
+            AddGiven(sub, sup) => {
+                self.data.givens.remove(&(sub, sup));
+            }
             AddCombination(Glb, ref regions) => {
                 self.glbs.remove(regions);
             }
@@ -466,6 +492,15 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
         self.undo_log.push(AddVerify(index));
     }
 
+    pub(super) fn add_given(&mut self, sub: Region<'tcx>, sup: ty::RegionVid) {
+        // cannot add givens once regions are resolved
+        if self.data.givens.insert((sub, sup)) {
+            debug!("add_given({:?} <= {:?})", sub, sup);
+
+            self.undo_log.push(AddGiven(sub, sup));
+        }
+    }
+
     pub(super) fn make_eqregion(
         &mut self,
         origin: SubregionOrigin<'tcx>,
@@ -769,8 +804,11 @@ impl<'tcx> RegionConstraintData<'tcx> {
     /// Returns `true` if this region constraint data contains no constraints, and `false`
     /// otherwise.
     pub fn is_empty(&self) -> bool {
-        let RegionConstraintData { constraints, member_constraints, verifys } = self;
-        constraints.is_empty() && member_constraints.is_empty() && verifys.is_empty()
+        let RegionConstraintData { constraints, member_constraints, verifys, givens } = self;
+        constraints.is_empty()
+            && member_constraints.is_empty()
+            && verifys.is_empty()
+            && givens.is_empty()
     }
 }
 

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -388,6 +388,7 @@ fn resolve_negative_obligation<'tcx>(
     let wf_tys = ocx.assumed_wf_types(param_env, DUMMY_SP, body_def_id);
     let outlives_env = OutlivesEnvironment::with_bounds(
         param_env,
+        Some(&infcx),
         infcx.implied_bounds_tys(param_env, body_def_id, wf_tys),
     );
 

compiler/rustc_trait_selection/src/traits/misc.rs

@@ -111,6 +111,7 @@ pub fn type_allowed_to_implement_copy<'tcx>(
             // Check regions assuming the self type of the impl is WF
             let outlives_env = OutlivesEnvironment::with_bounds(
                 param_env,
+                Some(&infcx),
                 infcx.implied_bounds_tys(
                     param_env,
                     parent_cause.body_id,

compiler/rustc_trait_selection/src/traits/outlives_bounds.rs

@@ -3,8 +3,7 @@ use crate::traits::query::type_op::{self, TypeOp, TypeOpOutput};
 use crate::traits::query::NoSolution;
 use crate::traits::ObligationCause;
 use rustc_data_structures::fx::FxIndexSet;
-use rustc_infer::infer::resolve::OpportunisticRegionResolver;
-use rustc_middle::ty::{self, ParamEnv, Ty, TypeFolder, TypeVisitableExt};
+use rustc_middle::ty::{self, ParamEnv, Ty};
 use rustc_span::def_id::LocalDefId;
 
 pub use rustc_middle::traits::query::OutlivesBound;
@@ -53,10 +52,6 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
         body_id: LocalDefId,
         ty: Ty<'tcx>,
     ) -> Vec<OutlivesBound<'tcx>> {
-        let ty = self.resolve_vars_if_possible(ty);
-        let ty = OpportunisticRegionResolver::new(self).fold_ty(ty);
-        assert!(!ty.needs_infer());
-
         let span = self.tcx.def_span(body_id);
         let result = param_env
             .and(type_op::implied_outlives_bounds::ImpliedOutlivesBounds { ty })
@@ -110,7 +105,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
         tys: FxIndexSet<Ty<'tcx>>,
     ) -> Bounds<'a, 'tcx> {
         tys.into_iter()
-            .map(move |ty| self.implied_outlives_bounds(param_env, body_id, ty))
+            .map(move |ty| {
+                let ty = self.resolve_vars_if_possible(ty);
+                self.implied_outlives_bounds(param_env, body_id, ty)
+            })
             .flatten()
     }
 }