From 1a663c0f53c71cbf69a982f699fbb00cdfce48f8 Mon Sep 17 00:00:00 2001 From: Jack Huey <31162821+jackh726@users.noreply.github.com> Date: Sun, 11 Sep 2022 03:53:54 -0400 Subject: [PATCH 1/2] Cleanup free_region_relations a bit --- .../rustc_borrowck/src/region_infer/values.rs | 1 + .../src/type_check/free_region_relations.rs | 152 ++++++++++-------- compiler/rustc_borrowck/src/type_check/mod.rs | 2 + .../rustc_infer/src/infer/outlives/verify.rs | 1 + .../src/implied_outlives_bounds.rs | 1 + tests/ui/nll/issue-52057.rs | 4 +- 6 files changed, 90 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/values.rs b/compiler/rustc_borrowck/src/region_infer/values.rs index c3dfeedc205f7..6a3748fded554 100644 --- a/compiler/rustc_borrowck/src/region_infer/values.rs +++ b/compiler/rustc_borrowck/src/region_infer/values.rs @@ -187,6 +187,7 @@ pub(crate) struct PlaceholderIndices { } impl PlaceholderIndices { + /// Returns the `PlaceholderIndex` for the inserted `PlaceholderRegion` pub(crate) fn insert(&mut self, placeholder: ty::PlaceholderRegion) -> PlaceholderIndex { let (index, _) = self.indices.insert_full(placeholder); index.into() diff --git a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs index 82ff862479e81..f0d964ae78fcd 100644 --- a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs +++ b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs @@ -8,6 +8,7 @@ use rustc_infer::infer::InferCtxt; use rustc_middle::mir::ConstraintCategory; use rustc_middle::traits::query::OutlivesBound; use rustc_middle::ty::{self, RegionVid, Ty}; +use rustc_span::Span; use rustc_trait_selection::traits::query::type_op::{self, TypeOp}; use std::rc::Rc; use type_op::TypeOpOutput; @@ -217,8 +218,28 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { self.inverse_outlives.add(fr_b, fr_a); } + #[instrument(level = "debug", skip(self))] pub(crate) fn create(mut self) -> CreateResult<'tcx> { let span = self.infcx.tcx.def_span(self.universal_regions.defining_ty.def_id()); + + // Insert the facts we know from the predicates. Why? Why not. + let param_env = self.param_env; + self.add_outlives_bounds(outlives::explicit_outlives_bounds(param_env)); + + // Finally: + // - outlives is reflexive, so `'r: 'r` for every region `'r` + // - `'static: 'r` for every region `'r` + // - `'r: 'fn_body` for every (other) universally quantified + // region `'r`, all of which are provided by our caller + let fr_static = self.universal_regions.fr_static; + let fr_fn_body = self.universal_regions.fr_fn_body; + for fr in self.universal_regions.universal_regions() { + debug!("build: relating free region {:?} to itself and to 'static", fr); + self.relate_universal_regions(fr, fr); + self.relate_universal_regions(fr_static, fr); + self.relate_universal_regions(fr, fr_fn_body); + } + let unnormalized_input_output_tys = self .universal_regions .unnormalized_input_tys @@ -236,78 +257,52 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { // the `relations` is built. let mut normalized_inputs_and_output = Vec::with_capacity(self.universal_regions.unnormalized_input_tys.len() + 1); - let constraint_sets: Vec<_> = unnormalized_input_output_tys - .flat_map(|ty| { - debug!("build: input_or_output={:?}", ty); - // We add implied bounds from both the unnormalized and normalized ty. - // See issue #87748 - let constraints_implied1 = self.add_implied_bounds(ty); - let TypeOpOutput { output: norm_ty, constraints: constraints1, .. } = self - .param_env - .and(type_op::normalize::Normalize::new(ty)) - .fully_perform(self.infcx) - .unwrap_or_else(|_| { - let reported = self - .infcx - .tcx - .sess - .delay_span_bug(span, &format!("failed to normalize {:?}", ty)); - TypeOpOutput { - output: self.infcx.tcx.ty_error_with_guaranteed(reported), - constraints: None, - error_info: None, - } - }); - // Note: we need this in examples like - // ``` - // trait Foo { - // type Bar; - // fn foo(&self) -> &Self::Bar; - // } - // impl Foo for () { - // type Bar = (); - // fn foo(&self) -> &() {} - // } - // ``` - // Both &Self::Bar and &() are WF - let constraints_implied2 = - if ty != norm_ty { self.add_implied_bounds(norm_ty) } else { None }; - normalized_inputs_and_output.push(norm_ty); - constraints1.into_iter().chain(constraints_implied1).chain(constraints_implied2) - }) - .collect(); - - // Insert the facts we know from the predicates. Why? Why not. - let param_env = self.param_env; - self.add_outlives_bounds(outlives::explicit_outlives_bounds(param_env)); + let mut constraints = vec![]; + for ty in unnormalized_input_output_tys { + debug!("build: input_or_output={:?}", ty); + // We add implied bounds from both the unnormalized and normalized ty. + // See issue #87748 + let constraints_unnorm = self.add_implied_bounds(ty); + constraints_unnorm.map(|c| constraints.push(c)); + let TypeOpOutput { output: norm_ty, constraints: constraints_normalize, .. } = self + .param_env + .and(type_op::normalize::Normalize::new(ty)) + .fully_perform(self.infcx) + .unwrap_or_else(|_| { + self.infcx + .tcx + .sess + .delay_span_bug(span, &format!("failed to normalize {:?}", ty)); + TypeOpOutput { + output: self.infcx.tcx.ty_error(), + constraints: None, + error_info: None, + } + }); + constraints_normalize.map(|c| constraints.push(c)); + + // Note: we need this in examples like + // ``` + // trait Foo { + // type Bar; + // fn foo(&self) -> &Self::Bar; + // } + // impl Foo for () { + // type Bar = (); + // fn foo(&self) ->&() {} + // } + // ``` + // Both &Self::Bar and &() are WF + if ty != norm_ty { + let constraints_norm = self.add_implied_bounds(norm_ty); + constraints_norm.map(|c| constraints.push(c)); + } - // Finally: - // - outlives is reflexive, so `'r: 'r` for every region `'r` - // - `'static: 'r` for every region `'r` - // - `'r: 'fn_body` for every (other) universally quantified - // region `'r`, all of which are provided by our caller - let fr_static = self.universal_regions.fr_static; - let fr_fn_body = self.universal_regions.fr_fn_body; - for fr in self.universal_regions.universal_regions() { - debug!("build: relating free region {:?} to itself and to 'static", fr); - self.relate_universal_regions(fr, fr); - self.relate_universal_regions(fr_static, fr); - self.relate_universal_regions(fr, fr_fn_body); + normalized_inputs_and_output.push(norm_ty); } - for data in &constraint_sets { - constraint_conversion::ConstraintConversion::new( - self.infcx, - &self.universal_regions, - &self.region_bound_pairs, - self.implicit_region_bound, - self.param_env, - Locations::All(span), - span, - ConstraintCategory::Internal, - &mut self.constraints, - ) - .convert_all(data); + for c in constraints { + self.push_region_constraints(c, span); } CreateResult { @@ -321,6 +316,24 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { } } + #[instrument(skip(self, data), level = "debug")] + fn push_region_constraints(&mut self, data: &QueryRegionConstraints<'tcx>, span: Span) { + debug!("constraints generated: {:#?}", data); + + constraint_conversion::ConstraintConversion::new( + self.infcx, + &self.universal_regions, + &self.region_bound_pairs, + self.implicit_region_bound, + self.param_env, + Locations::All(span), + span, + ConstraintCategory::Internal, + &mut self.constraints, + ) + .convert_all(data); + } + /// Update the type of a single local, which should represent /// either the return type of the MIR or one of its arguments. At /// the same time, compute and add any implied bounds that come @@ -332,6 +345,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { .and(type_op::implied_outlives_bounds::ImpliedOutlivesBounds { ty }) .fully_perform(self.infcx) .unwrap_or_else(|_| bug!("failed to compute implied bounds {:?}", ty)); + debug!(?bounds, ?constraints); self.add_outlives_bounds(bounds); constraints } diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 5b52846562f87..8273113295c02 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -910,6 +910,8 @@ pub(crate) struct MirTypeckRegionConstraints<'tcx> { } impl<'tcx> MirTypeckRegionConstraints<'tcx> { + /// Creates a `Region` that for a given `PlaceholderRegion`, or returns the + /// region that corresponds to a previously created one. fn placeholder_region( &mut self, infcx: &InferCtxt<'tcx>, diff --git a/compiler/rustc_infer/src/infer/outlives/verify.rs b/compiler/rustc_infer/src/infer/outlives/verify.rs index 94de9bc2d0228..bae246418b05a 100644 --- a/compiler/rustc_infer/src/infer/outlives/verify.rs +++ b/compiler/rustc_infer/src/infer/outlives/verify.rs @@ -207,6 +207,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// /// In some cases, such as when `erased_ty` represents a `ty::Param`, however, /// the result is precise. + #[instrument(level = "debug", skip(self))] fn declared_generic_bounds_from_env_for_erased_ty( &self, erased_ty: Ty<'tcx>, diff --git a/compiler/rustc_traits/src/implied_outlives_bounds.rs b/compiler/rustc_traits/src/implied_outlives_bounds.rs index fe633d687d91b..e2fb80d51caed 100644 --- a/compiler/rustc_traits/src/implied_outlives_bounds.rs +++ b/compiler/rustc_traits/src/implied_outlives_bounds.rs @@ -81,6 +81,7 @@ fn compute_implied_outlives_bounds<'tcx>( // From the full set of obligations, just filter down to the // region relationships. outlives_bounds.extend(obligations.into_iter().filter_map(|obligation| { + debug!(?obligation); assert!(!obligation.has_escaping_bound_vars()); match obligation.predicate.kind().no_bound_vars() { None => None, diff --git a/tests/ui/nll/issue-52057.rs b/tests/ui/nll/issue-52057.rs index 98f49fe8f5507..5991c1104c8a2 100644 --- a/tests/ui/nll/issue-52057.rs +++ b/tests/ui/nll/issue-52057.rs @@ -1,6 +1,6 @@ // Regression test for #52057. There is an implied bound -// that `I: 'a` where `'a` is the lifetime of `self` in `parse_first`; -// but to observe that, one must normalize first. +// that `I: 'x` where `'x` is the lifetime of the reference `&mut Self::Input` +// in `parse_first`; but to observe that, one must normalize first. // // run-pass From 0637b6b4718bcaa273b55f83ca4be893a86b1f83 Mon Sep 17 00:00:00 2001 From: Jack Huey <31162821+jackh726@users.noreply.github.com> Date: Sun, 11 Sep 2022 04:27:25 -0400 Subject: [PATCH 2/2] Update implied_outlives_bounds to properly register implied bounds behind normalization --- .../src/type_check/free_region_relations.rs | 13 ++- compiler/rustc_borrowck/src/type_check/mod.rs | 2 +- .../src/implied_outlives_bounds.rs | 85 +++++++++++-------- compiler/rustc_traits/src/lib.rs | 1 + 4 files changed, 60 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs index f0d964ae78fcd..2dd24fe034038 100644 --- a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs +++ b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs @@ -226,7 +226,6 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { let param_env = self.param_env; self.add_outlives_bounds(outlives::explicit_outlives_bounds(param_env)); - // Finally: // - outlives is reflexive, so `'r: 'r` for every region `'r` // - `'static: 'r` for every region `'r` // - `'r: 'fn_body` for every (other) universally quantified @@ -263,7 +262,9 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { // We add implied bounds from both the unnormalized and normalized ty. // See issue #87748 let constraints_unnorm = self.add_implied_bounds(ty); - constraints_unnorm.map(|c| constraints.push(c)); + if let Some(c) = constraints_unnorm { + constraints.push(c) + } let TypeOpOutput { output: norm_ty, constraints: constraints_normalize, .. } = self .param_env .and(type_op::normalize::Normalize::new(ty)) @@ -279,7 +280,9 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { error_info: None, } }); - constraints_normalize.map(|c| constraints.push(c)); + if let Some(c) = constraints_normalize { + constraints.push(c) + } // Note: we need this in examples like // ``` @@ -295,7 +298,9 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { // Both &Self::Bar and &() are WF if ty != norm_ty { let constraints_norm = self.add_implied_bounds(norm_ty); - constraints_norm.map(|c| constraints.push(c)); + if let Some(c) = constraints_norm { + constraints.push(c) + } } normalized_inputs_and_output.push(norm_ty); diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 8273113295c02..64c96281ed983 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -910,7 +910,7 @@ pub(crate) struct MirTypeckRegionConstraints<'tcx> { } impl<'tcx> MirTypeckRegionConstraints<'tcx> { - /// Creates a `Region` that for a given `PlaceholderRegion`, or returns the + /// Creates a `Region` for a given `PlaceholderRegion`, or returns the /// region that corresponds to a previously created one. fn placeholder_region( &mut self, diff --git a/compiler/rustc_traits/src/implied_outlives_bounds.rs b/compiler/rustc_traits/src/implied_outlives_bounds.rs index e2fb80d51caed..2c6c77072e60e 100644 --- a/compiler/rustc_traits/src/implied_outlives_bounds.rs +++ b/compiler/rustc_traits/src/implied_outlives_bounds.rs @@ -70,48 +70,61 @@ fn compute_implied_outlives_bounds<'tcx>( let obligations = wf::obligations(ocx.infcx, param_env, CRATE_DEF_ID, 0, arg, DUMMY_SP) .unwrap_or_default(); - // While these predicates should all be implied by other parts of - // the program, they are still relevant as they may constrain - // inference variables, which is necessary to add the correct - // implied bounds in some cases, mostly when dealing with projections. - ocx.register_obligations( - obligations.iter().filter(|o| o.predicate.has_non_region_infer()).cloned(), - ); - - // From the full set of obligations, just filter down to the - // region relationships. - outlives_bounds.extend(obligations.into_iter().filter_map(|obligation| { + for obligation in obligations { debug!(?obligation); assert!(!obligation.has_escaping_bound_vars()); - match obligation.predicate.kind().no_bound_vars() { - None => None, - Some(pred) => match pred { - ty::PredicateKind::Clause(ty::Clause::Trait(..)) - | ty::PredicateKind::Subtype(..) - | ty::PredicateKind::Coerce(..) - | ty::PredicateKind::Clause(ty::Clause::Projection(..)) - | ty::PredicateKind::ClosureKind(..) - | ty::PredicateKind::ObjectSafe(..) - | ty::PredicateKind::ConstEvaluatable(..) - | ty::PredicateKind::ConstEquate(..) - | ty::PredicateKind::Ambiguous - | ty::PredicateKind::TypeWellFormedFromEnv(..) => None, - ty::PredicateKind::WellFormed(arg) => { - wf_args.push(arg); - None + + // While these predicates should all be implied by other parts of + // the program, they are still relevant as they may constrain + // inference variables, which is necessary to add the correct + // implied bounds in some cases, mostly when dealing with projections. + // + // Another important point here: we only register `Projection` + // predicates, since otherwise we might register outlives + // predicates containing inference variables, and we don't + // learn anything new from those. + if obligation.predicate.has_non_region_infer() { + match obligation.predicate.kind().skip_binder() { + ty::PredicateKind::Clause(ty::Clause::Projection(..)) => { + ocx.register_obligation(obligation.clone()); } + _ => {} + } + } - ty::PredicateKind::Clause(ty::Clause::RegionOutlives( - ty::OutlivesPredicate(r_a, r_b), - )) => Some(ty::OutlivesPredicate(r_a.into(), r_b)), + let pred = match obligation.predicate.kind().no_bound_vars() { + None => continue, + Some(pred) => pred, + }; + match pred { + ty::PredicateKind::Clause(ty::Clause::Trait(..)) + | ty::PredicateKind::Subtype(..) + | ty::PredicateKind::Coerce(..) + | ty::PredicateKind::Clause(ty::Clause::Projection(..)) + | ty::PredicateKind::ClosureKind(..) + | ty::PredicateKind::ObjectSafe(..) + | ty::PredicateKind::ConstEvaluatable(..) + | ty::PredicateKind::ConstEquate(..) + | ty::PredicateKind::Ambiguous + | ty::PredicateKind::TypeWellFormedFromEnv(..) => {} + + // We need to search through *all* WellFormed predicates + ty::PredicateKind::WellFormed(arg) => { + wf_args.push(arg); + } + + // We need to register region relationships + ty::PredicateKind::Clause(ty::Clause::RegionOutlives(ty::OutlivesPredicate( + r_a, + r_b, + ))) => outlives_bounds.push(ty::OutlivesPredicate(r_a.into(), r_b)), - ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate( - ty_a, - r_b, - ))) => Some(ty::OutlivesPredicate(ty_a.into(), r_b)), - }, + ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate( + ty_a, + r_b, + ))) => outlives_bounds.push(ty::OutlivesPredicate(ty_a.into(), r_b)), } - })); + } } // This call to `select_all_or_error` is necessary to constrain inference variables, which we diff --git a/compiler/rustc_traits/src/lib.rs b/compiler/rustc_traits/src/lib.rs index 9aa26667e7bf4..8bea5588ae75e 100644 --- a/compiler/rustc_traits/src/lib.rs +++ b/compiler/rustc_traits/src/lib.rs @@ -4,6 +4,7 @@ #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] #![feature(let_chains)] +#![feature(drain_filter)] #![recursion_limit = "256"] #[macro_use]