Auto merge of #151510 - LorrensP-2158466:wf-closure-arg, r=<try>

WF checks on closure arguments.

Author: bors

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -585,7 +585,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     fn check_type_tests(
         &self,
         infcx: &InferCtxt<'tcx>,
-        mut propagated_outlives_requirements: Option<&mut Vec<ClosureOutlivesRequirement<'tcx>>>,
+        propagated_outlives_requirements: Option<&mut Vec<ClosureOutlivesRequirement<'tcx>>>,
         errors_buffer: &mut RegionErrors<'tcx>,
     ) {
         let tcx = infcx.tcx;
@@ -595,6 +595,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // the user. Avoid that.
         let mut deduplicate_errors = FxIndexSet::default();
 
+        // Each type test introduces one or more OR-constraints (e.g. T: 'a OR T: 'b),
+        // where at least one option in each constraint must be satisfied. All such
+        // constraints must be satisfied simultaneously: i.e., they form a conjunction (AND).
+        // We'll use this conjunctive requirement later on.
+        let mut conjunctive_propagated_outlives_requirement =
+            propagated_outlives_requirements.is_some().then_some(vec![]);
+
         for type_test in &self.type_tests {
             debug!("check_type_test: {:?}", type_test);
 
@@ -608,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_requirement
+                && self.try_promote_type_test(
+                    infcx,
+                    type_test,
+                    conjunctive_propagated_outlives_requirements,
+                )
             {
                 continue;
             }
@@ -633,6 +645,85 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 errors_buffer.push(RegionErrorKind::TypeTestError { type_test: type_test.clone() });
             }
         }
+
+        if let Some(mut conjunctive_requirement) = conjunctive_propagated_outlives_requirement
+            && !conjunctive_requirement.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.
+            //
+            // Because of the outlives relations, we can actually have a stronger redundancy check,
+            // say we have following requirements that create a conjunctive requirement:
+            // R1: T: 'a
+            // R2: T: 'b OR T: 'c
+            // R: R1 AND R2
+            //
+            // And we have we an assumption in our environment that `'a: 'b`, we can thus remove R2
+            // as well. `T: 'b` is implied by `T: 'a` because of the assumption `'a: 'b`:
+            // T -> 'a -> 'b
+            //
+            // So we can filter redundant OR requirements with the following algorithm:
+            // Collect every Unit requirement. Then for every OR requirement, loop over its
+            // individual requirements and if the region is outlived by the region of one of the
+            // units, remove the entire OR requirement.
+
+            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_requirement
+                .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_requirement.retain(|or_requirement| {
+                or_requirement.len() == 1
+                    || !or_requirement.iter().any(|r| {
+                        let (ty, region) = requirement_key(*r);
+                        units.iter().any(|&(unit_subj, unit_region)| {
+                            // Same type, and the unit region outlives the disjunct region,
+                            // meaning T: unit_region implies T: region.
+                            unit_subj == ty
+                                && self.universal_region_relations.outlives(unit_region, region)
+                        })
+                    })
+            });
+
+            assert!(
+                !conjunctive_requirement.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_requirement.into_iter().flatten());
+        }
     }
 
     /// Invoked when we have some type-test (e.g., `T: 'X`) that we cannot
@@ -664,7 +755,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         &self,
         infcx: &InferCtxt<'tcx>,
         type_test: &TypeTest<'tcx>,
-        propagated_outlives_requirements: &mut Vec<ClosureOutlivesRequirement<'tcx>>,
+        propagated_outlives_requirements: &mut Vec<Vec<ClosureOutlivesRequirement<'tcx>>>,
     ) -> bool {
         let tcx = infcx.tcx;
         let TypeTest { generic_kind, lower_bound, span: blame_span, verify_bound: _ } = *type_test;
@@ -690,24 +781,41 @@ 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 mut found_outlived_universal_region = false;
-        for ur in self.scc_values.universal_regions_outlived_by(r_scc) {
-            found_outlived_universal_region = true;
+        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"
+        );
+
+        for ur in minimal_universal_regions {
             debug!("universal_region_outlived_by ur={:?}", ur);
             let non_local_ub = self.universal_region_relations.non_local_upper_bounds(ur);
             debug!(?non_local_ub);
@@ -716,6 +824,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));
@@ -727,14 +836,10 @@ 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
-        // also be at least as large as some universal region, as the type test is otherwise
-        // trivial.
-        assert!(found_outlived_universal_region);
         true
     }
 

compiler/rustc_borrowck/src/type_check/canonical.rs

@@ -3,7 +3,6 @@ use std::fmt;
 use rustc_errors::ErrorGuaranteed;
 use rustc_infer::infer::canonical::Canonical;
 use rustc_infer::infer::outlives::env::RegionBoundPairs;
-use rustc_middle::bug;
 use rustc_middle::mir::{Body, ConstraintCategory};
 use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, Unnormalized, Upcast};
 use rustc_span::Span;
@@ -259,53 +258,4 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                 .and(type_op::ascribe_user_type::AscribeUserType { mir_ty, user_ty }),
         );
     }
-
-    /// *Incorrectly* skips the WF checks we normally do in `ascribe_user_type`.
-    ///
-    /// FIXME(#104478, #104477): This is a hack for backward-compatibility.
-    #[instrument(skip(self), level = "debug")]
-    pub(super) fn ascribe_user_type_skip_wf(
-        &mut self,
-        mir_ty: Ty<'tcx>,
-        user_ty: ty::UserType<'tcx>,
-        span: Span,
-    ) {
-        let ty::UserTypeKind::Ty(user_ty) = user_ty.kind else { bug!() };
-
-        // A fast path for a common case with closure input/output types.
-        if let ty::Infer(_) = user_ty.kind() {
-            self.eq_types(user_ty, mir_ty, Locations::All(span), ConstraintCategory::Boring)
-                .unwrap();
-            return;
-        }
-
-        // This is a hack. `body.local_decls` are not necessarily normalized in the old
-        // solver due to not deeply normalizing in writeback. So we must re-normalize here.
-        //
-        // I am not sure of a test case where this actually matters. There is a similar
-        // hack in `equate_inputs_and_outputs` which does have associated test cases.
-        let mir_ty = match self.infcx.next_trait_solver() {
-            true => mir_ty,
-            false => self.normalize(Unnormalized::new_wip(mir_ty), Locations::All(span)),
-        };
-
-        let cause = ObligationCause::dummy_with_span(span);
-        let param_env = self.infcx.param_env;
-        let _: Result<_, ErrorGuaranteed> = self.fully_perform_op(
-            Locations::All(span),
-            ConstraintCategory::Boring,
-            type_op::custom::CustomTypeOp::new(
-                |ocx| {
-                    // The `AscribeUserType` query would normally emit a wf
-                    // obligation for the unnormalized user_ty here. This is
-                    // where the "incorrectly skips the WF checks we normally do"
-                    // happens
-                    let user_ty = ocx.normalize(&cause, param_env, Unnormalized::new_wip(user_ty));
-                    ocx.eq(&cause, param_env, user_ty, mir_ty)?;
-                    Ok(())
-                },
-                "ascribe_user_type_skip_wf",
-            ),
-        );
-    }
 }

compiler/rustc_borrowck/src/type_check/input_output.rs

@@ -118,7 +118,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                 .skip(1 + if is_coroutine_with_implicit_resume_ty { 1 } else { 0 })
                 .map(|local| &self.body.local_decls[local]),
         ) {
-            self.ascribe_user_type_skip_wf(
+            self.ascribe_user_type(
                 arg_decl.ty,
                 ty::UserType::new(ty::UserTypeKind::Ty(user_ty)),
                 arg_decl.source_info.span,
@@ -127,7 +127,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
 
         // If the user explicitly annotated the output type, enforce it.
         let output_decl = &self.body.local_decls[RETURN_PLACE];
-        self.ascribe_user_type_skip_wf(
+        self.ascribe_user_type(
             output_decl.ty,
             ty::UserType::new(ty::UserTypeKind::Ty(user_provided_sig.output())),
             output_decl.source_info.span,

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<I>` may not live long enough
         //~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box<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 the parameter type `I` may not live long enough
         //~| ERROR `f` does not live long enough
     }
 }

tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr

@@ -69,34 +69,6 @@ help: consider adding an explicit lifetime bound
 LL |     pub fn ack<I: 'static>(&mut self, f: impl for<'a> Fn(&'a usize) -> Box<I>) {
    |                 +++++++++
 
-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<I: 'static>(&mut self, f: impl for<'a> Fn(&'a usize) -> Box<I>) {
-   |                 +++++++++
-
-error[E0311]: the parameter type `I` may not live long enough
-  --> $DIR/unconstrained-closure-lifetime-generic.rs:10:35
-   |
-LL |     pub fn ack<I>(&mut self, f: impl for<'a> Fn(&'a usize) -> Box<I>) {
-   |                   --------- 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<I>) {
-   |                +++  ++++   ++
-
 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<dyn for<'a> 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`.

tests/ui/consts/issue-102117.rs

@@ -16,7 +16,6 @@ impl VTable {
                 layout: Layout::new::<T>(),
                 type_id: TypeId::of::<T>(),
                 //~^ ERROR the parameter type `T` may not live long enough
-                //~| ERROR the parameter type `T` may not live long enough
                 drop_in_place: unsafe {
                     transmute::<unsafe fn(*mut T), unsafe fn(*mut ())>(drop_in_place::<T>)
                 },

tests/ui/consts/issue-102117.stderr

@@ -12,21 +12,6 @@ help: consider adding an explicit lifetime bound
 LL |     pub fn new<T: 'static>() -> &'static Self {
    |                 +++++++++
 
-error[E0310]: the parameter type `T` may not live long enough
-  --> $DIR/issue-102117.rs:17:26
-   |
-LL |                 type_id: TypeId::of::<T>(),
-   |                          ^^^^^^^^^^^^^^^
-   |                          |
-   |                          the parameter type `T` must be valid for the static lifetime...
-   |                          ...so that the type `T` 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 new<T: 'static>() -> &'static Self {
-   |                 +++++++++
-
-error: aborting due to 2 previous errors
+error: aborting due to 1 previous error
 
 For more information about this error, try `rustc --explain E0310`.

tests/ui/implied-bounds/hrlt-implied-trait-bounds-roundtrip.rs

@@ -1,4 +1,3 @@
-//@ check-pass
 struct Foo<'a>(&'a ())
 where
     (): Trait<'a>;
@@ -21,7 +20,7 @@ where
 }
 
 fn main() {
-    let bar: for<'a, 'b> fn(Foo<'a>, &'b ()) = |_, _| {};
+    let bar: for<'a, 'b> fn(Foo<'a>, &'b ()) = |_, _| {}; //~ ERROR: lifetime may not live long enough
 
     // If `could_use_implied_bounds` were to use implied bounds,
     // keeping 'a late-bound, then we could assign that function

tests/ui/implied-bounds/hrlt-implied-trait-bounds-roundtrip.stderr

@@ -0,0 +1,11 @@
+error: lifetime may not live long enough
+  --> $DIR/hrlt-implied-trait-bounds-roundtrip.rs:23:49
+   |
+LL |     let bar: for<'a, 'b> fn(Foo<'a>, &'b ()) = |_, _| {};
+   |                                                 ^
+   |                                                 |
+   |                                                 has type `Foo<'1>`
+   |                                                 requires that `'1` must outlive `'static`
+
+error: aborting due to 1 previous error
+