Fix a bug (thanks @lcnr!) when a placeholder illegally outlives an existential

Author: amandasystems

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -161,6 +161,19 @@ pub(crate) enum RegionErrorKind<'tcx> {
         /// encountered and leave the rest unreported so as not to overwhelm the user.
         is_reported: bool,
     },
+    /// Indicates that a placeholder has a universe too large for one
+    /// of its member existentials, or, equivalently, that there is
+    /// a path through the outlives constraint graph from a placeholder
+    /// to an existential region that cannot name it.
+    PlaceholderReachesExistentialThatCannotNameIt {
+        /// the placeholder that transitively outlives an
+        /// existential that shouldn't leak into it
+        longer_fr: RegionVid,
+        /// The existential leaking into `longer_fr`.
+        existental_that_cannot_name_longer: RegionVid,
+        // `longer_fr`'s originating placeholder region.
+        placeholder: ty::PlaceholderRegion,
+    },
 }
 
 /// Information about the various region constraints involved in a borrow checker error.
@@ -349,15 +362,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
 
         for (nll_error, _) in nll_errors.into_iter() {
             match nll_error {
-                // A type-test failed and the constraint was rewritten due
-                // to higher-ranked trait bounds.
                 RegionErrorKind::TypeTestError {
                     generic_kind,
                     lower_bound,
                     span,
                     failed_due_to_placeholders: true,
                 } => emit_generic_does_not_live_long_enough(self, generic_kind, span, lower_bound),
-
                 RegionErrorKind::TypeTestError {
                     lower_bound,
                     span,
@@ -396,7 +406,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         lower_bound_region,
                     ));
                 }
-
                 RegionErrorKind::UnexpectedHiddenRegion { span, hidden_ty, key, member_region } => {
                     let named_ty =
                         self.regioncx.name_regions_for_member_constraint(self.infcx.tcx, hidden_ty);
@@ -420,33 +429,19 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         diag.delay_as_bug();
                     }
                 }
-
                 RegionErrorKind::BoundUniversalRegionError {
                     longer_fr,
                     placeholder,
                     error_element,
                 } => {
                     let error_vid = self.regioncx.region_from_element(longer_fr, &error_element);
 
-                    // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
-                    let (_, cause) = self.regioncx.find_outlives_blame_span(
+                    self.report_erroneous_rvid_reaches_placeholder(
                         longer_fr,
-                        NllRegionVariableOrigin::Placeholder(placeholder),
-                        error_vid,
-                    );
-
-                    // FIXME these methods should have better names, and also probably not be this generic.
-                    // FIXME note that we *throw away* the error element here! We probably want to
-                    // thread it through the computation further down and use it, but there currently isn't
-                    // anything there to receive it.
-                    self.regioncx.universe_info(placeholder.universe).report_erroneous_element(
-                        self,
                         placeholder,
-                        cause,
-                        None,
+                        error_vid,
                     );
                 }
-
                 RegionErrorKind::PlaceholderMismatch { rvid_a, rvid_b, origin_a, origin_b } => {
                     debug!(
                         "Placeholder mismatch: {rvid_a:?} ({origin_a:?}) reaches {rvid_b:?} ({origin_b:?})"
@@ -468,7 +463,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         Some(origin_b),
                     );
                 }
-
                 RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => {
                     if is_reported {
                         self.report_region_error(
@@ -490,6 +484,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         );
                     }
                 }
+                RegionErrorKind::PlaceholderReachesExistentialThatCannotNameIt {
+                    longer_fr,
+                    existental_that_cannot_name_longer,
+                    placeholder,
+                } => self.report_erroneous_rvid_reaches_placeholder(
+                    longer_fr,
+                    placeholder,
+                    existental_that_cannot_name_longer,
+                ),
             }
         }
 

compiler/rustc_borrowck/src/eliminate_placeholders.rs

@@ -112,6 +112,9 @@ struct RegionTracker {
 
     // Metadata about reachable placeholders
     reachable_placeholders: PlaceholderReachability,
+
+    // Track the existential with the smallest universe we reach.
+    min_universe_reachable_existential: Option<(UniverseIndex, RegionVid)>,
 }
 
 impl scc::Annotation for RegionTracker {
@@ -122,14 +125,25 @@ impl scc::Annotation for RegionTracker {
         } else {
             self.min_universe
         };
+
+        let min_universe_reachable_existential = smallest_reachable_existential(
+            self.min_universe_reachable_existential,
+            other.min_universe_reachable_existential,
+        );
         Self {
             reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders),
             min_universe,
             representative: self.representative.merge_scc(other.representative),
+            min_universe_reachable_existential,
         }
     }
 
     fn merge_reached(mut self, other: Self) -> Self {
+        self.min_universe_reachable_existential = smallest_reachable_existential(
+            self.min_universe_reachable_existential,
+            other.min_universe_reachable_existential,
+        );
+
         // This detail is subtle. We stop early here, because there may be multiple
         // illegally reached universes, but they are not equally good as blame candidates.
         // In general, the ones with the smallest indices of their RegionVids will
@@ -150,14 +164,25 @@ impl scc::Annotation for RegionTracker {
     }
 }
 
+fn smallest_reachable_existential(
+    min_universe_reachable_existential_1: Option<(UniverseIndex, RegionVid)>,
+    min_universe_reachable_existential_2: Option<(UniverseIndex, RegionVid)>,
+) -> Option<(UniverseIndex, RegionVid)> {
+    match (min_universe_reachable_existential_1, min_universe_reachable_existential_2) {
+        (Some(a), Some(b)) => Some(std::cmp::min(a, b)),
+        (a, b) => a.or(b),
+    }
+}
+
 impl RegionTracker {
     fn new(representative: RegionVid, definition: &RegionDefinition<'_>) -> Self {
         let universe_and_rvid = (definition.universe, representative);
-        let (representative, reachable_placeholders) = {
+        let (representative, reachable_placeholders, min_universe_reachable_existential) = {
             match definition.origin {
                 NllRegionVariableOrigin::FreeRegion => (
                     Representative::FreeRegion(representative),
                     PlaceholderReachability::NoPlaceholders,
+                    None,
                 ),
                 NllRegionVariableOrigin::Placeholder(_) => (
                     Representative::Placeholder(representative),
@@ -166,14 +191,21 @@ impl RegionTracker {
                         min_placeholder: representative,
                         max_placeholder: representative,
                     },
+                    None,
                 ),
                 NllRegionVariableOrigin::Existential { .. } => (
                     Representative::Existential(representative),
                     PlaceholderReachability::NoPlaceholders,
+                    Some((definition.universe, representative)),
                 ),
             }
         };
-        Self { representative, min_universe: universe_and_rvid, reachable_placeholders }
+        Self {
+            representative,
+            min_universe: universe_and_rvid,
+            reachable_placeholders,
+            min_universe_reachable_existential,
+        }
     }
 
     /// The smallest-indexed universe reachable from and/or in this SCC.
@@ -237,6 +269,16 @@ impl RegionTracker {
 
         Some((max_u_rvid, max_u))
     }
+
+    /// Check for the second and final type of placeholder leak,
+    /// where an existential `'e` outlives (transitively) a placeholder `p`
+    /// and `e` cannot name `p`.
+    ///
+    /// Returns *a* culprit (though there may be more than one).
+    fn reaches_existential_that_cannot_name_us(&self) -> Option<RegionVid> {
+        let (min_u, min_rvid) = self.min_universe_reachable_existential?;
+        (min_u < self.min_universe()).then_some(min_rvid)
+    }
 }
 
 impl scc::Annotations<RegionVid, ConstraintSccIndex, RegionTracker>
@@ -279,10 +321,19 @@ fn find_placeholder_mismatch_errors<'tcx>(
         };
 
         let scc = sccs.scc(rvid);
+        let annotation = annotations.scc_to_annotation[scc];
 
-        let Some(other_placeholder) =
-            annotations.scc_to_annotation[scc].reaches_other_placeholder(rvid)
-        else {
+        if let Some(existental_that_cannot_name_rvid) =
+            annotation.reaches_existential_that_cannot_name_us()
+        {
+            errors_buffer.push(RegionErrorKind::PlaceholderReachesExistentialThatCannotNameIt {
+                longer_fr: rvid,
+                existental_that_cannot_name_longer: existental_that_cannot_name_rvid,
+                placeholder: origin_a,
+            })
+        }
+
+        let Some(other_placeholder) = annotation.reaches_other_placeholder(rvid) else {
             trace!("{rvid:?} reaches no other placeholders");
             continue;
         };
@@ -486,6 +537,7 @@ fn rewrite_outlives<'tcx>(
     for scc in sccs.all_sccs() {
         let annotation: RegionTracker = annotations.scc_to_annotation[scc];
         if scc == sccs.scc(fr_static) {
+            trace!("Skipping adding 'static: 'static.");
             // No use adding 'static: 'static.
             continue;
         }
@@ -501,7 +553,10 @@ fn rewrite_outlives<'tcx>(
 
         let min_u = annotation.min_universe();
 
-        debug!("Universe {max_u:?} is too large for its SCC!");
+        debug!(
+            "Universe {max_u:?} is too large for its SCC, represented by {:?}",
+            annotation.representative
+        );
         let blame_to = if annotation.representative.rvid() == max_u_rvid {
             // We originally had a large enough universe to fit all our reachable
             // placeholders, but had it lowered because we also absorbed something

compiler/rustc_borrowck/src/lib.rs

@@ -2502,6 +2502,33 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
             tcx.emit_node_span_lint(UNUSED_MUT, lint_root, span, VarNeedNotMut { span: mut_span })
         }
     }
+
+    /// Report that longer_fr: shorter_fr, which doesn't hold,
+    /// where longer_fr is a placeholder from `placeholder`.
+    fn report_erroneous_rvid_reaches_placeholder(
+        &mut self,
+        longer_fr: RegionVid,
+        placeholder: ty::Placeholder<ty::BoundRegion>,
+        error_vid: RegionVid,
+    ) {
+        // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
+        let (_, cause) = self.regioncx.find_outlives_blame_span(
+            longer_fr,
+            NllRegionVariableOrigin::Placeholder(placeholder),
+            error_vid,
+        );
+
+        // FIXME these methods should have better names, and also probably not be this generic.
+        // FIXME note that we *throw away* the error element here! We probably want to
+        // thread it through the computation further down and use it, but there currently isn't
+        // anything there to receive it.
+        self.regioncx.universe_info(placeholder.universe).report_erroneous_element(
+            self,
+            placeholder,
+            cause,
+            None,
+        );
+    }
 }
 
 /// The degree of overlap between 2 places for borrow-checking.