Rollup merge of rust-lang#116960 - lqd:applied-member-constraints-scope, r=matthewjasper

Location-insensitive polonius: consider a loan escaping if an SCC has member constraints applied only

The location-insensitive analysis considered loans to escape if there were member constraints, which makes *some* sense for scopes and matches the scopes that NLL computes on all the tests.

However, polonius and NLLs differ on the fuzzed case rust-lang#116657, where an SCC has member constraints but no applied ones (and is kinda surprising). The existing UI tests with member constraints impacting scopes all have some constraint applied.

This PR changes the location-insensitive analysis to consider a loan to escape if there are applied member constraints, and for extra paranoia/insurance via fuzzing and crater: actually checks the constraint's min choice is indeed a universal region as we expect. (This could be turned into a `debug_assert` and early return as a slight optimization after these periods of verification)

The 4 UI tests where member constraints are meaningful for computing scopes still pass obviously, and this also fixes rust-lang#116657.

r? `@matthewjasper`

Author: matthiaskrgr

compiler/rustc_borrowck/src/dataflow.rs

@@ -272,21 +272,28 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
         loan_issued_at: Location,
     ) {
         let sccs = self.regioncx.constraint_sccs();
+        let universal_regions = self.regioncx.universal_regions();
         let issuing_region_scc = sccs.scc(issuing_region);
 
         // We first handle the cases where the loan doesn't go out of scope, depending on the issuing
         // region's successors.
         for scc in sccs.depth_first_search(issuing_region_scc) {
-            // 1. Via member constraints
+            // 1. Via applied member constraints
             //
             // The issuing region can flow into the choice regions, and they are either:
             // - placeholders or free regions themselves,
             // - or also transitively outlive a free region.
             //
-            // That is to say, if there are member constraints here, the loan escapes the function
-            // and cannot go out of scope. We can early return.
-            if self.regioncx.scc_has_member_constraints(scc) {
-                return;
+            // That is to say, if there are applied member constraints here, the loan escapes the
+            // function and cannot go out of scope. We could early return here.
+            //
+            // For additional insurance via fuzzing and crater, we verify that the constraint's min
+            // choice indeed escapes the function. In the future, we could e.g. turn this check into
+            // a debug assert and early return as an optimization.
+            for constraint in self.regioncx.applied_member_constraints(scc) {
+                if universal_regions.is_universal_region(constraint.min_choice) {
+                    return;
+                }
             }
 
             // 2. Via regions that are live at all points: placeholders and free regions.
@@ -413,12 +420,12 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
             let mut polonius_prec = PoloniusOutOfScopePrecomputer::new(body, regioncx);
             for (loan_idx, loan_data) in borrow_set.iter_enumerated() {
                 let issuing_region = loan_data.region;
-                let issued_location = loan_data.reserve_location;
+                let loan_issued_at = loan_data.reserve_location;
 
                 polonius_prec.precompute_loans_out_of_scope(
                     loan_idx,
                     issuing_region,
-                    issued_location,
+                    loan_issued_at,
                 );
             }
 

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -644,11 +644,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.scc_universes[scc]
     }
 
-    /// Once region solving has completed, this function will return
-    /// the member constraints that were applied to the value of a given
-    /// region `r`. See `AppliedMemberConstraint`.
-    pub(crate) fn applied_member_constraints(&self, r: RegionVid) -> &[AppliedMemberConstraint] {
-        let scc = self.constraint_sccs.scc(r);
+    /// Once region solving has completed, this function will return the member constraints that
+    /// were applied to the value of a given SCC `scc`. See `AppliedMemberConstraint`.
+    pub(crate) fn applied_member_constraints(
+        &self,
+        scc: ConstraintSccIndex,
+    ) -> &[AppliedMemberConstraint] {
         binary_search_util::binary_search_slice(
             &self.member_constraints_applied,
             |applied| applied.member_region_scc,
@@ -1945,7 +1946,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             // Member constraints can also give rise to `'r: 'x` edges that
             // were not part of the graph initially, so watch out for those.
             // (But they are extremely rare; this loop is very cold.)
-            for constraint in self.applied_member_constraints(r) {
+            for constraint in self.applied_member_constraints(self.constraint_sccs.scc(r)) {
                 let p_c = &self.member_constraints[constraint.member_constraint_index];
                 let constraint = OutlivesConstraint {
                     sup: r,
@@ -2292,11 +2293,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.constraint_sccs.as_ref()
     }
 
-    /// Returns whether the given SCC has any member constraints.
-    pub(crate) fn scc_has_member_constraints(&self, scc: ConstraintSccIndex) -> bool {
-        self.member_constraints.indices(scc).next().is_some()
-    }
-
     /// Returns whether the given SCC is live at all points: whether the representative is a
     /// placeholder or a free region.
     pub(crate) fn scc_is_live_at_all_points(&self, scc: ConstraintSccIndex) -> bool {

compiler/rustc_session/src/config.rs

@@ -3463,9 +3463,10 @@ impl DumpMonoStatsFormat {
 
 /// `-Zpolonius` values, enabling the borrow checker polonius analysis, and which version: legacy,
 /// or future prototype.
-#[derive(Clone, Copy, PartialEq, Hash, Debug)]
+#[derive(Clone, Copy, PartialEq, Hash, Debug, Default)]
 pub enum Polonius {
     /// The default value: disabled.
+    #[default]
     Off,
 
     /// Legacy version, using datalog and the `polonius-engine` crate. Historical value for `-Zpolonius`.
@@ -3475,12 +3476,6 @@ pub enum Polonius {
     Next,
 }
 
-impl Default for Polonius {
-    fn default() -> Self {
-        Polonius::Off
-    }
-}
-
 impl Polonius {
     /// Returns whether the legacy version of polonius is enabled
     pub fn is_legacy_enabled(&self) -> bool {

tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.nll.stderr

@@ -0,0 +1,36 @@
+error[E0046]: not all trait items implemented, missing: `call`
+  --> $DIR/location-insensitive-scopes-issue-116657.rs:18:1
+   |
+LL |     fn call(x: Self) -> Self::Output;
+   |     --------------------------------- `call` from trait
+...
+LL | impl<T: PlusOne> Callable for T {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `call` in implementation
+
+error: unconstrained opaque type
+  --> $DIR/location-insensitive-scopes-issue-116657.rs:22:19
+   |
+LL |     type Output = impl PlusOne;
+   |                   ^^^^^^^^^^^^
+   |
+   = note: `Output` must be used in combination with a concrete type within the same impl
+
+error[E0700]: hidden type for `impl PlusOne` captures lifetime that does not appear in bounds
+  --> $DIR/location-insensitive-scopes-issue-116657.rs:28:5
+   |
+LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne {
+   |         --                     ------------ opaque type defined here
+   |         |
+   |         hidden type `<&'a mut i32 as Callable>::Output` captures the lifetime `'a` as defined here
+LL |     <&mut i32 as Callable>::call(y)
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: to declare that `impl PlusOne` captures `'a`, you can add an explicit `'a` lifetime bound
+   |
+LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne + 'a {
+   |                                             ++++
+
+error: aborting due to 3 previous errors
+
+Some errors have detailed explanations: E0046, E0700.
+For more information about an error, try `rustc --explain E0046`.

tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.polonius.stderr

@@ -0,0 +1,36 @@
+error[E0046]: not all trait items implemented, missing: `call`
+  --> $DIR/location-insensitive-scopes-issue-116657.rs:18:1
+   |
+LL |     fn call(x: Self) -> Self::Output;
+   |     --------------------------------- `call` from trait
+...
+LL | impl<T: PlusOne> Callable for T {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `call` in implementation
+
+error: unconstrained opaque type
+  --> $DIR/location-insensitive-scopes-issue-116657.rs:22:19
+   |
+LL |     type Output = impl PlusOne;
+   |                   ^^^^^^^^^^^^
+   |
+   = note: `Output` must be used in combination with a concrete type within the same impl
+
+error[E0700]: hidden type for `impl PlusOne` captures lifetime that does not appear in bounds
+  --> $DIR/location-insensitive-scopes-issue-116657.rs:28:5
+   |
+LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne {
+   |         --                     ------------ opaque type defined here
+   |         |
+   |         hidden type `<&'a mut i32 as Callable>::Output` captures the lifetime `'a` as defined here
+LL |     <&mut i32 as Callable>::call(y)
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: to declare that `impl PlusOne` captures `'a`, you can add an explicit `'a` lifetime bound
+   |
+LL | fn test<'a>(y: &'a mut i32) -> impl PlusOne + 'a {
+   |                                             ++++
+
+error: aborting due to 3 previous errors
+
+Some errors have detailed explanations: E0046, E0700.
+For more information about an error, try `rustc --explain E0046`.

tests/ui/nll/polonius/location-insensitive-scopes-issue-116657.rs

@@ -0,0 +1,33 @@
+// This is a non-regression test for issue #116657, where NLL and `-Zpolonius=next` computed
+// different loan scopes when a member constraint was not ultimately applied.
+
+// revisions: nll polonius
+// [polonius] compile-flags: -Zpolonius=next
+
+#![feature(impl_trait_in_assoc_type)]
+
+trait Callable {
+    type Output;
+    fn call(x: Self) -> Self::Output;
+}
+
+trait PlusOne {}
+
+impl<'a> PlusOne for &'a mut i32 {}
+
+impl<T: PlusOne> Callable for T {
+    //[nll]~^ ERROR not all trait items implemented
+    //[polonius]~^^ ERROR not all trait items implemented
+
+    type Output = impl PlusOne;
+    //[nll]~^ ERROR unconstrained opaque type
+    //[polonius]~^^ ERROR unconstrained opaque type
+}
+
+fn test<'a>(y: &'a mut i32) -> impl PlusOne {
+    <&mut i32 as Callable>::call(y)
+    //[nll]~^ ERROR hidden type for `impl PlusOne` captures lifetime
+    //[polonius]~^^ ERROR hidden type for `impl PlusOne` captures lifetime
+}
+
+fn main() {}