Auto merge of #99501 - lcnr:check-regions-infcx, r=oli-obk

move `considering_regions` to the infcx

it seems weird to prove some obligations which constrain inference vars while ignoring regions  in a context which considers regions. This is especially weird because even for a fulfillment context with ignored regions, we still added region outlives bounds when directly relating regions.

tbh our handling of regions is still very weird, but at least this is a step in the right direction imo.

r? rust-lang/types

Author: bors

compiler/rustc_error_codes/src/error_codes.rs

@@ -559,7 +559,7 @@ E0790: include_str!("./error_codes/E0790.md"),
 //  E0273, // on_unimplemented #1
 //  E0274, // on_unimplemented #2
 //  E0278, // requirement is not satisfied
-    E0279, // requirement is not satisfied
+//  E0279,
     E0280, // requirement is not satisfied
 //  E0285, // overflow evaluation builtin bounds
 //  E0296, // replaced with a generic attribute input check

compiler/rustc_infer/src/infer/at.rs

@@ -65,6 +65,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         Self {
             tcx: self.tcx,
             defining_use_anchor: self.defining_use_anchor,
+            considering_regions: self.considering_regions,
             in_progress_typeck_results: self.in_progress_typeck_results,
             inner: self.inner.clone(),
             skip_leak_check: self.skip_leak_check.clone(),

compiler/rustc_infer/src/infer/mod.rs

@@ -265,6 +265,11 @@ pub struct InferCtxt<'a, 'tcx> {
     /// might come up during inference or typeck.
     pub defining_use_anchor: DefiningAnchor,
 
+    /// Whether this inference context should care about region obligations in
+    /// the root universe. Most notably, this is used during hir typeck as region
+    /// solving is left to borrowck instead.
+    pub considering_regions: bool,
+
     /// During type-checking/inference of a body, `in_progress_typeck_results`
     /// contains a reference to the typeck results being built up, which are
     /// used for reading closure kinds/signatures as they are inferred,
@@ -539,8 +544,9 @@ impl<'tcx> fmt::Display for FixupError<'tcx> {
 /// without using `Rc` or something similar.
 pub struct InferCtxtBuilder<'tcx> {
     tcx: TyCtxt<'tcx>,
-    fresh_typeck_results: Option<RefCell<ty::TypeckResults<'tcx>>>,
     defining_use_anchor: DefiningAnchor,
+    considering_regions: bool,
+    fresh_typeck_results: Option<RefCell<ty::TypeckResults<'tcx>>>,
 }
 
 pub trait TyCtxtInferExt<'tcx> {
@@ -552,6 +558,7 @@ impl<'tcx> TyCtxtInferExt<'tcx> for TyCtxt<'tcx> {
         InferCtxtBuilder {
             tcx: self,
             defining_use_anchor: DefiningAnchor::Error,
+            considering_regions: true,
             fresh_typeck_results: None,
         }
     }
@@ -577,6 +584,11 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
         self
     }
 
+    pub fn ignoring_regions(mut self) -> Self {
+        self.considering_regions = false;
+        self
+    }
+
     /// Given a canonical value `C` as a starting point, create an
     /// inference context that contains each of the bound values
     /// within instantiated as a fresh variable. The `f` closure is
@@ -601,11 +613,17 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
     }
 
     pub fn enter<R>(&mut self, f: impl for<'a> FnOnce(InferCtxt<'a, 'tcx>) -> R) -> R {
-        let InferCtxtBuilder { tcx, defining_use_anchor, ref fresh_typeck_results } = *self;
+        let InferCtxtBuilder {
+            tcx,
+            defining_use_anchor,
+            considering_regions,
+            ref fresh_typeck_results,
+        } = *self;
         let in_progress_typeck_results = fresh_typeck_results.as_ref();
         f(InferCtxt {
             tcx,
             defining_use_anchor,
+            considering_regions,
             in_progress_typeck_results,
             inner: RefCell::new(InferCtxtInner::new()),
             lexical_region_resolutions: RefCell::new(None),
@@ -1043,16 +1061,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         &self,
         cause: &traits::ObligationCause<'tcx>,
         predicate: ty::PolyRegionOutlivesPredicate<'tcx>,
-    ) -> UnitResult<'tcx> {
-        self.commit_if_ok(|_snapshot| {
-            let ty::OutlivesPredicate(r_a, r_b) =
-                self.replace_bound_vars_with_placeholders(predicate);
-            let origin = SubregionOrigin::from_obligation_cause(cause, || {
-                RelateRegionParamBound(cause.span)
-            });
-            self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
-            Ok(())
-        })
+    ) {
+        let ty::OutlivesPredicate(r_a, r_b) = self.replace_bound_vars_with_placeholders(predicate);
+        let origin =
+            SubregionOrigin::from_obligation_cause(cause, || RelateRegionParamBound(cause.span));
+        self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
     }
 
     /// Number of type variables created so far.

compiler/rustc_trait_selection/src/traits/auto_trait.rs

@@ -793,9 +793,7 @@ impl<'tcx> AutoTraitFinder<'tcx> {
                 }
                 ty::PredicateKind::RegionOutlives(binder) => {
                     let binder = bound_predicate.rebind(binder);
-                    if select.infcx().region_outlives_predicate(&dummy_cause, binder).is_err() {
-                        return false;
-                    }
+                    select.infcx().region_outlives_predicate(&dummy_cause, binder)
                 }
                 ty::PredicateKind::TypeOutlives(binder) => {
                     let binder = bound_predicate.rebind(binder);

compiler/rustc_trait_selection/src/traits/codegen.rs

@@ -30,7 +30,9 @@ pub fn codegen_fulfill_obligation<'tcx>(
 
     // Do the initial selection for the obligation. This yields the
     // shallow result we are looking for -- that is, what specific impl.
-    tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).enter(|infcx| {
+    let mut infcx_builder =
+        tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble);
+    infcx_builder.enter(|infcx| {
         //~^ HACK `Bubble` is required for
         // this test to pass: type-alias-impl-trait/assoc-projection-ice.rs
         let mut selcx = SelectionContext::new(&infcx);

compiler/rustc_trait_selection/src/traits/engine.rs

@@ -15,8 +15,6 @@ use rustc_middle::ty::{self, Ty, TyCtxt};
 
 pub trait TraitEngineExt<'tcx> {
     fn new(tcx: TyCtxt<'tcx>) -> Box<Self>;
-
-    fn new_ignoring_regions(tcx: TyCtxt<'tcx>) -> Box<Self>;
 }
 
 impl<'tcx> TraitEngineExt<'tcx> for dyn TraitEngine<'tcx> {
@@ -27,14 +25,6 @@ impl<'tcx> TraitEngineExt<'tcx> for dyn TraitEngine<'tcx> {
             Box::new(FulfillmentContext::new())
         }
     }
-
-    fn new_ignoring_regions(tcx: TyCtxt<'tcx>) -> Box<Self> {
-        if tcx.sess.opts.unstable_opts.chalk {
-            Box::new(ChalkFulfillmentContext::new())
-        } else {
-            Box::new(FulfillmentContext::new_ignoring_regions())
-        }
-    }
 }
 
 /// Used if you want to have pleasant experience when dealing

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -789,24 +789,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                         span_bug!(span, "coerce requirement gave wrong error: `{:?}`", predicate)
                     }
 
-                    ty::PredicateKind::RegionOutlives(predicate) => {
-                        let predicate = bound_predicate.rebind(predicate);
-                        let predicate = self.resolve_vars_if_possible(predicate);
-                        let err = self
-                            .region_outlives_predicate(&obligation.cause, predicate)
-                            .err()
-                            .unwrap();
-                        struct_span_err!(
-                            self.tcx.sess,
-                            span,
-                            E0279,
-                            "the requirement `{}` is not satisfied (`{}`)",
-                            predicate,
-                            err,
-                        )
-                    }
-
-                    ty::PredicateKind::Projection(..) | ty::PredicateKind::TypeOutlives(..) => {
+                    ty::PredicateKind::RegionOutlives(..)
+                    | ty::PredicateKind::Projection(..)
+                    | ty::PredicateKind::TypeOutlives(..) => {
                         let predicate = self.resolve_vars_if_possible(obligation.predicate);
                         struct_span_err!(
                             self.tcx.sess,

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -58,19 +58,6 @@ pub struct FulfillmentContext<'tcx> {
 
     relationships: FxHashMap<ty::TyVid, ty::FoundRelationships>,
 
-    // Should this fulfillment context register type-lives-for-region
-    // obligations on its parent infcx? In some cases, region
-    // obligations are either already known to hold (normalization) or
-    // hopefully verified elsewhere (type-impls-bound), and therefore
-    // should not be checked.
-    //
-    // Note that if we are normalizing a type that we already
-    // know is well-formed, there should be no harm setting this
-    // to true - all the region variables should be determinable
-    // using the RFC 447 rules, which don't depend on
-    // type-lives-for-region constraints, and because the type
-    // is well-formed, the constraints should hold.
-    register_region_obligations: bool,
     // Is it OK to register obligations into this infcx inside
     // an infcx snapshot?
     //
@@ -103,7 +90,6 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
             relationships: FxHashMap::default(),
-            register_region_obligations: true,
             usable_in_snapshot: false,
         }
     }
@@ -112,30 +98,18 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
             relationships: FxHashMap::default(),
-            register_region_obligations: true,
             usable_in_snapshot: true,
         }
     }
 
-    pub fn new_ignoring_regions() -> FulfillmentContext<'tcx> {
-        FulfillmentContext {
-            predicates: ObligationForest::new(),
-            relationships: FxHashMap::default(),
-            register_region_obligations: false,
-            usable_in_snapshot: false,
-        }
-    }
-
     /// Attempts to select obligations using `selcx`.
     fn select(&mut self, selcx: &mut SelectionContext<'a, 'tcx>) -> Vec<FulfillmentError<'tcx>> {
         let span = debug_span!("select", obligation_forest_size = ?self.predicates.len());
         let _enter = span.enter();
 
         // Process pending obligations.
-        let outcome: Outcome<_, _> = self.predicates.process_obligations(&mut FulfillProcessor {
-            selcx,
-            register_region_obligations: self.register_region_obligations,
-        });
+        let outcome: Outcome<_, _> =
+            self.predicates.process_obligations(&mut FulfillProcessor { selcx });
 
         // FIXME: if we kept the original cache key, we could mark projection
         // obligations as complete for the projection cache here.
@@ -239,7 +213,6 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
 
 struct FulfillProcessor<'a, 'b, 'tcx> {
     selcx: &'a mut SelectionContext<'b, 'tcx>,
-    register_region_obligations: bool,
 }
 
 fn mk_pending(os: Vec<PredicateObligation<'_>>) -> Vec<PendingPredicateObligation<'_>> {
@@ -385,19 +358,16 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
                 }
 
                 ty::PredicateKind::RegionOutlives(data) => {
-                    match infcx.region_outlives_predicate(&obligation.cause, Binder::dummy(data)) {
-                        Ok(()) => ProcessResult::Changed(vec![]),
-                        Err(_) => ProcessResult::Error(CodeSelectionError(Unimplemented)),
+                    if infcx.considering_regions || data.has_placeholders() {
+                        infcx.region_outlives_predicate(&obligation.cause, Binder::dummy(data));
                     }
+
+                    ProcessResult::Changed(vec![])
                 }
 
                 ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(t_a, r_b)) => {
-                    if self.register_region_obligations {
-                        self.selcx.infcx().register_region_obligation_with_cause(
-                            t_a,
-                            r_b,
-                            &obligation.cause,
-                        );
+                    if infcx.considering_regions {
+                        infcx.register_region_obligation_with_cause(t_a, r_b, &obligation.cause);
                     }
                     ProcessResult::Changed(vec![])
                 }

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -163,7 +163,7 @@ pub fn type_known_to_meet_bound_modulo_regions<'a, 'tcx>(
         // The handling of regions in this area of the code is terrible,
         // see issue #29149. We should be able to improve on this with
         // NLL.
-        let mut fulfill_cx = FulfillmentContext::new_ignoring_regions();
+        let mut fulfill_cx = FulfillmentContext::new();
 
         // We can use a dummy node-id here because we won't pay any mind
         // to region obligations that arise (there shouldn't really be any
@@ -207,21 +207,21 @@ fn do_normalize_predicates<'tcx>(
     predicates: Vec<ty::Predicate<'tcx>>,
 ) -> Result<Vec<ty::Predicate<'tcx>>, ErrorGuaranteed> {
     let span = cause.span;
-    tcx.infer_ctxt().enter(|infcx| {
-        // FIXME. We should really... do something with these region
-        // obligations. But this call just continues the older
-        // behavior (i.e., doesn't cause any new bugs), and it would
-        // take some further refactoring to actually solve them. In
-        // particular, we would have to handle implied bounds
-        // properly, and that code is currently largely confined to
-        // regionck (though I made some efforts to extract it
-        // out). -nmatsakis
-        //
-        // @arielby: In any case, these obligations are checked
-        // by wfcheck anyway, so I'm not sure we have to check
-        // them here too, and we will remove this function when
-        // we move over to lazy normalization *anyway*.
-        let fulfill_cx = FulfillmentContext::new_ignoring_regions();
+    // FIXME. We should really... do something with these region
+    // obligations. But this call just continues the older
+    // behavior (i.e., doesn't cause any new bugs), and it would
+    // take some further refactoring to actually solve them. In
+    // particular, we would have to handle implied bounds
+    // properly, and that code is currently largely confined to
+    // regionck (though I made some efforts to extract it
+    // out). -nmatsakis
+    //
+    // @arielby: In any case, these obligations are checked
+    // by wfcheck anyway, so I'm not sure we have to check
+    // them here too, and we will remove this function when
+    // we move over to lazy normalization *anyway*.
+    tcx.infer_ctxt().ignoring_regions().enter(|infcx| {
+        let fulfill_cx = FulfillmentContext::new();
         let predicates =
             match fully_normalize(&infcx, fulfill_cx, cause, elaborated_env, predicates) {
                 Ok(predicates) => predicates,

compiler/rustc_trait_selection/src/traits/specialize/mod.rs

@@ -207,18 +207,7 @@ fn fulfill_implication<'a, 'tcx>(
     // (which are packed up in penv)
 
     infcx.save_and_restore_in_snapshot_flag(|infcx| {
-        // If we came from `translate_substs`, we already know that the
-        // predicates for our impl hold (after all, we know that a more
-        // specialized impl holds, so our impl must hold too), and
-        // we only want to process the projections to determine the
-        // the types in our substs using RFC 447, so we can safely
-        // ignore region obligations, which allows us to avoid threading
-        // a node-id to assign them with.
-        //
-        // If we came from specialization graph construction, then
-        // we already make a mockery out of the region system, so
-        // why not ignore them a bit earlier?
-        let mut fulfill_cx = FulfillmentContext::new_ignoring_regions();
+        let mut fulfill_cx = FulfillmentContext::new();
         for oblig in obligations.chain(more_obligations) {
             fulfill_cx.register_predicate_obligation(&infcx, oblig);
         }

compiler/rustc_typeck/src/check/inherited.rs

@@ -86,7 +86,10 @@ impl<'tcx> Inherited<'_, 'tcx> {
         let hir_owner = tcx.hir().local_def_id_to_hir_id(def_id).owner;
 
         InheritedBuilder {
-            infcx: tcx.infer_ctxt().with_fresh_in_progress_typeck_results(hir_owner),
+            infcx: tcx
+                .infer_ctxt()
+                .ignoring_regions()
+                .with_fresh_in_progress_typeck_results(hir_owner),
             def_id,
         }
     }
@@ -113,7 +116,7 @@ impl<'a, 'tcx> Inherited<'a, 'tcx> {
                 maybe_typeck_results: infcx.in_progress_typeck_results,
             },
             infcx,
-            fulfillment_cx: RefCell::new(<dyn TraitEngine<'_>>::new_ignoring_regions(tcx)),
+            fulfillment_cx: RefCell::new(<dyn TraitEngine<'_>>::new(tcx)),
             locals: RefCell::new(Default::default()),
             deferred_sized_obligations: RefCell::new(Vec::new()),
             deferred_call_resolutions: RefCell::new(Default::default()),

compiler/rustc_typeck/src/check/mod.rs

@@ -87,10 +87,10 @@ mod op;
 mod pat;
 mod place_op;
 mod region;
-mod regionck;
+pub mod regionck;
 pub mod rvalue_scopes;
 mod upvar;
-mod wfcheck;
+pub mod wfcheck;
 pub mod writeback;
 
 use check::{check_abi, check_fn, check_mod_item_types};

compiler/rustc_typeck/src/check/wfcheck.rs

@@ -1913,7 +1913,7 @@ impl<'a, 'tcx> WfCheckingCtxt<'a, 'tcx> {
     }
 }
 
-pub(super) fn impl_implied_bounds<'tcx>(
+pub fn impl_implied_bounds<'tcx>(
     tcx: TyCtxt<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     impl_def_id: LocalDefId,

compiler/rustc_typeck/src/coherence/builtin.rs

@@ -116,8 +116,8 @@ fn visit_implementation_of_copy(tcx: TyCtxt<'_>, impl_did: LocalDefId) {
                 // why this field does not implement Copy. This is useful because sometimes
                 // it is not immediately clear why Copy is not implemented for a field, since
                 // all we point at is the field itself.
-                tcx.infer_ctxt().enter(|infcx| {
-                    let mut fulfill_cx = traits::FulfillmentContext::new_ignoring_regions();
+                tcx.infer_ctxt().ignoring_regions().enter(|infcx| {
+                    let mut fulfill_cx = traits::FulfillmentContext::new();
                     fulfill_cx.register_bound(
                         &infcx,
                         param_env,