diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 5cdda777723b3..0668fe32cc070 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -589,7 +589,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fn check_type_tests( &self, infcx: &InferCtxt<'tcx>, - mut propagated_outlives_requirements: Option<&mut Vec>>, + propagated_outlives_requirements: Option<&mut Vec>>, errors_buffer: &mut RegionErrors<'tcx>, ) { let tcx = infcx.tcx; @@ -599,6 +599,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { // the user. Avoid that. let mut deduplicate_errors = FxIndexSet::default(); + let mut conjunctive_propagated_outlives_requirements = + propagated_outlives_requirements.is_some().then_some(vec![]); + for type_test in &self.type_tests { debug!("check_type_test: {:?}", type_test); @@ -612,8 +615,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { continue; } - if let Some(propagated_outlives_requirements) = &mut propagated_outlives_requirements - && self.try_promote_type_test(infcx, type_test, propagated_outlives_requirements) + if let Some(conjunctive_propagated_outlives_requirements) = + &mut conjunctive_propagated_outlives_requirements + && self.try_promote_type_test( + infcx, + type_test, + conjunctive_propagated_outlives_requirements, + ) { continue; } @@ -637,6 +645,69 @@ impl<'tcx> RegionInferenceContext<'tcx> { errors_buffer.push(RegionErrorKind::TypeTestError { type_test: type_test.clone() }); } } + + if let Some(mut conjunctive_requirements) = conjunctive_propagated_outlives_requirements + && !conjunctive_requirements.is_empty() + { + // We can simplify this list of list of requirements. + // + // Say we did some number of type tests and it results in following requirements: + // + // R1: (T: 'a OR T: 'b) + // R2: (T: 'a) + // + // * See `try_promote_type_test` below on why we obtain OR requirements implicitly. + // + // Full requirement is then: R1 AND R2. *BUT*, we can remove R1 entirely, because we already + // require `T: 'a`, which implies `T:'a OR T: 'b`, making R1 redundant. + // + // The requirements can be seen as a boolean conjunctive normal form expression: + // Treat a requirement `T: 'region` as a boolean value, then this problem is (almost) + // equivalent to "Unit Propagation". However, this problem we are trying to solve is much + // simpler: Unit Propagation considers any form of subexpression, even containing negation + // of values, making it a multi-pass algorithm. The only subexpressions we encounter are of + // the form (R1 OR ... OR RN), thus if even on R is required on their own (a unit), this + // whole subexpression can be removed. + // + // So we can filter redundant OR requirements with the following algorithm: + // Collect every Unit requirement. Then for every other requirement, if one of the units is + // contained within them we can remove the entire requirement from the list. + + fn requirement_key<'a>(subject: ClosureOutlivesRequirement<'a>) -> (Ty<'a>, RegionVid) { + let ClosureOutlivesSubject::Ty(ClosureOutlivesSubjectTy { inner: ty }) = + subject.subject + else { + unreachable!("ClosureOutliveSubject of a type test is always a Ty"); + }; + (ty, subject.outlived_free_region) + } + + let units: Vec<_> = conjunctive_requirements + .iter() + .filter_map(|r| { + let [r] = r.as_slice() else { return None }; + Some(requirement_key(*r)) + }) + .collect(); + + // Remove the `or_requirement`s that contain any of the unit requirements. + conjunctive_requirements.retain(|or_requirement| { + or_requirement.len() == 1 + || !or_requirement.iter().any(|r| { + let key = requirement_key(*r); + units.contains(&key) + }) + }); + + assert!( + !conjunctive_requirements.is_empty(), + "It should not be possible to remove every requirement." + ); + // Propagate all requirements as is. + propagated_outlives_requirements + .expect("conjunctive_requirements is `Some`, so this should be as well") + .extend(conjunctive_requirements.into_iter().flatten()); + } } /// Invoked when we have some type-test (e.g., `T: 'X`) that we cannot @@ -668,7 +739,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'tcx>, type_test: &TypeTest<'tcx>, - propagated_outlives_requirements: &mut Vec>, + propagated_outlives_requirements: &mut Vec>>, ) -> bool { let tcx = infcx.tcx; let TypeTest { generic_kind, lower_bound, span: blame_span, verify_bound: _ } = *type_test; @@ -694,23 +765,42 @@ impl<'tcx> RegionInferenceContext<'tcx> { if let Some(p) = self.scc_values.placeholders_contained_in(r_scc).next() { debug!("encountered placeholder in higher universe: {:?}, requiring 'static", p); let static_r = self.universal_regions().fr_static; - propagated_outlives_requirements.push(ClosureOutlivesRequirement { + propagated_outlives_requirements.push(vec![ClosureOutlivesRequirement { subject, outlived_free_region: static_r, blame_span, category: ConstraintCategory::Boring, - }); + }]); // we can return here -- the code below might push add'l constraints // but they would all be weaker than this one. return true; } - // For each region outlived by lower_bound find a non-local, - // universal region (it may be the same region) and add it to - // `ClosureOutlivesRequirement`. + let universal_regions: Vec<_> = + self.scc_values.universal_regions_outlived_by(r_scc).collect(); + debug!(?universal_regions); + + // Filter to only the "minimal" universal regions: + // Drop any region `a` that strictly outlives another region `b`. + let minimal_universal_regions: Vec<_> = universal_regions + .iter() + .copied() + .filter(|&a| { + !universal_regions.iter().copied().any(|b| { + !self.universal_region_relations.outlives(a, b) + && self.universal_region_relations.outlives(b, a) + }) + }) + .collect(); + + assert!( + !minimal_universal_regions.is_empty(), + "There should always be at least 1 minimal region" + ); + let mut found_outlived_universal_region = false; - for ur in self.scc_values.universal_regions_outlived_by(r_scc) { + for ur in minimal_universal_regions { found_outlived_universal_region = true; debug!("universal_region_outlived_by ur={:?}", ur); let non_local_ub = self.universal_region_relations.non_local_upper_bounds(ur); @@ -720,6 +810,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // and `'3: '1` we only need to prove that T: '2 *or* T: '3, but to // avoid potential non-determinism we approximate this by requiring // T: '1 and T: '2. + let mut or_requirements = Vec::with_capacity(non_local_ub.len()); for upper_bound in non_local_ub { debug_assert!(self.universal_regions().is_universal_region(upper_bound)); debug_assert!(!self.universal_regions().is_local_free_region(upper_bound)); @@ -731,8 +822,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { category: ConstraintCategory::Boring, }; debug!(?requirement, "adding closure requirement"); - propagated_outlives_requirements.push(requirement); + or_requirements.push(requirement); } + propagated_outlives_requirements.push(or_requirements); } // If we succeed to promote the subject, i.e. it only contains non-local regions, // and fail to prove the type test inside of the closure, the `lower_bound` has to diff --git a/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs index 4fdf5470feac6..dcba2081cfb33 100644 --- a/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs +++ b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.rs @@ -13,8 +13,6 @@ impl Foo { //~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough //~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box` may not live long enough //~| ERROR the parameter type `I` may not live long enough - //~| ERROR the parameter type `I` may not live long enough - //~| ERROR the parameter type `I` may not live long enough //~| ERROR `f` does not live long enough } } diff --git a/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr index df86ce79f09c7..9befce0e94dc3 100644 --- a/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr +++ b/tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr @@ -69,34 +69,6 @@ help: consider adding an explicit lifetime bound LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { | +++++++++ -error[E0310]: the parameter type `I` may not live long enough - --> $DIR/unconstrained-closure-lifetime-generic.rs:10:35 - | -LL | self.bar = Box::new(|baz| Box::new(f(baz))); - | ^^^^^^^^^^^^^^^^ - | | - | the parameter type `I` must be valid for the static lifetime... - | ...so that the type `I` will meet its required lifetime bounds - | - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -help: consider adding an explicit lifetime bound - | -LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { - | +++++++++ - -error[E0311]: the parameter type `I` may not live long enough - --> $DIR/unconstrained-closure-lifetime-generic.rs:10:35 - | -LL | pub fn ack(&mut self, f: impl for<'a> Fn(&'a usize) -> Box) { - | --------- the parameter type `I` must be valid for the anonymous lifetime defined here... -LL | self.bar = Box::new(|baz| Box::new(f(baz))); - | ^^^^^^^^^^^^^^^^ ...so that the type `I` will meet its required lifetime bounds - | -help: consider adding an explicit lifetime bound - | -LL | pub fn ack<'a, I: 'a>(&'a mut self, f: impl for<'a> Fn(&'a usize) -> Box) { - | +++ ++++ ++ - error[E0597]: `f` does not live long enough --> $DIR/unconstrained-closure-lifetime-generic.rs:10:44 | @@ -113,7 +85,7 @@ LL | } | = note: due to object lifetime defaults, `Box Fn(&'a usize) -> Box<(dyn Any + 'a)>>` actually means `Box<(dyn for<'a> Fn(&'a usize) -> Box<(dyn Any + 'a)> + 'static)>` -error: aborting due to 8 previous errors +error: aborting due to 6 previous errors -Some errors have detailed explanations: E0310, E0311, E0597. +Some errors have detailed explanations: E0310, E0597. For more information about an error, try `rustc --explain E0310`. diff --git a/tests/ui/nll/closure-requirements/type-test-issue-154267.rs b/tests/ui/nll/closure-requirements/type-test-issue-154267.rs new file mode 100644 index 0000000000000..6137f38886437 --- /dev/null +++ b/tests/ui/nll/closure-requirements/type-test-issue-154267.rs @@ -0,0 +1,26 @@ +//@ check-pass +// This test checks that the compiler doesn't propagate `T: 'b` during the `T: 'a` type test. +// If it did, it would fail to compile, even though the program is sound. + +struct Arg<'a: 'c, 'b: 'c, 'c, T> { + field: *mut (&'a (), &'b (), &'c (), T), +} + +impl<'a, 'b, 'c, T> Arg<'a, 'b, 'c, T> { + fn constrain(self) + where + T: 'a, + T: 'c, + { + } +} + +fn takes_closure<'a, 'b, T>(_: impl for<'c> FnOnce(Arg<'a, 'b, 'c, T>)) {} + +// We have `'a: 'c` and `'b: 'c`, requiring `T: 'a` in `constrain` should not need +// `T: 'b` here. +fn error<'a, 'b, T: 'a>() { + takes_closure::<'a, 'b, T>(|arg| arg.constrain()); +} + +fn main() {}