From 451912262f69731d6481394b197bb284b8c07b33 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Thu, 5 Jun 2025 13:04:31 +0200 Subject: [PATCH 1/7] Region inference: Use outlives-static constraints in constraint search Revise the extra `r: 'static` constraints added upon universe issues to add an explanation, and use that explanation during constraint blame search. This greatly simplifies the region inference logic, which now does not need to reverse-engineer the event that caused a region to outlive 'static. --- .../src/diagnostics/explain_borrow.rs | 2 +- .../src/diagnostics/opaque_suggestions.rs | 7 +- .../src/diagnostics/region_errors.rs | 21 +- .../rustc_borrowck/src/handle_placeholders.rs | 282 ++++++++++++++---- .../src/region_infer/graphviz.rs | 3 + .../rustc_borrowck/src/region_infer/mod.rs | 179 ++++------- compiler/rustc_middle/src/mir/query.rs | 13 +- tests/ui/nll/ice-106874.stderr | 37 +-- ...missing-universe-cause-issue-114907.stderr | 8 +- 9 files changed, 343 insertions(+), 209 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index c4b0f503664ee..16f1b8be0cdcd 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -569,7 +569,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { let (blame_constraint, path) = self.regioncx.best_blame_constraint( borrow_region, NllRegionVariableOrigin::FreeRegion, - |r| self.regioncx.provides_universal_region(r, borrow_region, outlived_region), + outlived_region, ); let BlameConstraint { category, from_closure, cause, .. } = blame_constraint; diff --git a/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs b/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs index 7192a889adcbc..1d553d3a0b486 100644 --- a/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs +++ b/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs @@ -174,10 +174,9 @@ impl<'tcx> TypeVisitor> for FindOpaqueRegion<'_, 'tcx> { let opaque_region_vid = self.regioncx.to_region_vid(opaque_region); // Find a path between the borrow region and our opaque capture. - if let Some((path, _)) = - self.regioncx.find_constraint_paths_between_regions(self.borrow_region, |r| { - r == opaque_region_vid - }) + if let Some(path) = self + .regioncx + .constraint_path_between_regions(self.borrow_region, opaque_region_vid) { for constraint in path { // If we find a call in this path, then check if it defines the opaque. diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index 3bec07afa0fe0..829247c4d8885 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -62,7 +62,7 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> { | ConstraintCategory::Boring | ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal - | ConstraintCategory::IllegalUniverse => "", + | ConstraintCategory::IllegalPlaceholder(..) => "", } } } @@ -396,11 +396,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { 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( - longer_fr, - NllRegionVariableOrigin::Placeholder(placeholder), - error_vid, - ); + let cause = self + .regioncx + .best_blame_constraint( + longer_fr, + NllRegionVariableOrigin::Placeholder(placeholder), + error_vid, + ) + .0 + .cause; let universe = placeholder.universe; let universe_info = self.regioncx.universe_info(universe); @@ -456,9 +460,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ) { debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr); - let (blame_constraint, path) = self.regioncx.best_blame_constraint(fr, fr_origin, |r| { - self.regioncx.provides_universal_region(r, fr, outlived_fr) - }); + let (blame_constraint, path) = + self.regioncx.best_blame_constraint(fr, fr_origin, outlived_fr); let BlameConstraint { category, cause, variance_info, .. } = blame_constraint; debug!("report_region_error: category={:?} {:?} {:?}", category, cause, variance_info); diff --git a/compiler/rustc_borrowck/src/handle_placeholders.rs b/compiler/rustc_borrowck/src/handle_placeholders.rs index aaaf2f45c869d..fcb7d794f814e 100644 --- a/compiler/rustc_borrowck/src/handle_placeholders.rs +++ b/compiler/rustc_borrowck/src/handle_placeholders.rs @@ -1,6 +1,8 @@ //! Logic for lowering higher-kinded outlives constraints //! (with placeholders and universes) and turn them into regular //! outlives constraints. +use std::cell::OnceCell; +use std::collections::VecDeque; use rustc_data_structures::frozen::Frozen; use rustc_data_structures::fx::FxIndexMap; @@ -8,10 +10,12 @@ use rustc_data_structures::graph::scc; use rustc_data_structures::graph::scc::Sccs; use rustc_index::IndexVec; use rustc_infer::infer::RegionVariableOrigin; +use rustc_middle::bug; use rustc_middle::mir::ConstraintCategory; use rustc_middle::ty::{RegionVid, UniverseIndex}; -use tracing::debug; +use tracing::{debug, trace}; +use crate::constraints::graph::{ConstraintGraph, Normal, RegionGraph}; use crate::constraints::{ConstraintSccIndex, OutlivesConstraintSet}; use crate::consumers::OutlivesConstraint; use crate::diagnostics::UniverseInfo; @@ -64,18 +68,68 @@ impl scc::Annotations for SccAnnotations<'_, '_, RegionTracker> { type SccIdx = ConstraintSccIndex; } +#[derive(Copy, Debug, Clone, PartialEq, Eq)] +enum PlaceholderReachability { + /// This SCC reaches no placeholders. + NoPlaceholders, + /// This SCC reaches at least one placeholder. + Placeholders { + /// The largest-universed placeholder we can reach + max_universe: (UniverseIndex, RegionVid), + + /// The placeholder with the smallest ID + min_placeholder: RegionVid, + + /// The placeholder with the largest ID + max_placeholder: RegionVid, + }, +} + +impl PlaceholderReachability { + fn merge(self, other: PlaceholderReachability) -> PlaceholderReachability { + use PlaceholderReachability::*; + match (self, other) { + (NoPlaceholders, NoPlaceholders) => NoPlaceholders, + (NoPlaceholders, p @ Placeholders { .. }) + | (p @ Placeholders { .. }, NoPlaceholders) => p, + ( + Placeholders { + min_placeholder: min_pl, + max_placeholder: max_pl, + max_universe: max_u, + }, + Placeholders { min_placeholder, max_placeholder, max_universe }, + ) => Placeholders { + min_placeholder: core::cmp::min(min_pl, min_placeholder), + max_placeholder: core::cmp::max(max_pl, max_placeholder), + max_universe: core::cmp::max(max_u, max_universe), + }, + } + } + + /// If we have reached placeholders, determine if they can + /// be named from this universe. + fn can_be_named_by(&self, from: UniverseIndex) -> bool { + if let PlaceholderReachability::Placeholders { max_universe: (max_universe, _), .. } = self + { + from.can_name(*max_universe) + } else { + true // No placeholders, no problems. + } + } +} + /// An annotation for region graph SCCs that tracks /// the values of its elements. This annotates a single SCC. #[derive(Copy, Debug, Clone)] pub(crate) struct RegionTracker { - /// The largest universe of a placeholder reached from this SCC. - /// This includes placeholders within this SCC. - max_placeholder_universe_reached: UniverseIndex, + reachable_placeholders: PlaceholderReachability, /// The largest universe nameable from this SCC. /// It is the smallest nameable universes of all - /// existential regions reachable from it. - max_nameable_universe: UniverseIndex, + /// existential regions reachable from it. Earlier regions in the constraint graph are + /// preferred. + max_nameable_universe: (UniverseIndex, RegionVid), /// The representative Region Variable Id for this SCC. pub(crate) representative: Representative, @@ -83,16 +137,20 @@ pub(crate) struct RegionTracker { impl RegionTracker { pub(crate) fn new(rvid: RegionVid, definition: &RegionDefinition<'_>) -> Self { - let placeholder_universe = + let reachable_placeholders = if matches!(definition.origin, NllRegionVariableOrigin::Placeholder(_)) { - definition.universe + PlaceholderReachability::Placeholders { + max_universe: (definition.universe, rvid), + min_placeholder: rvid, + max_placeholder: rvid, + } } else { - UniverseIndex::ROOT + PlaceholderReachability::NoPlaceholders }; Self { - max_placeholder_universe_reached: placeholder_universe, - max_nameable_universe: definition.universe, + reachable_placeholders, + max_nameable_universe: (definition.universe, rvid), representative: Representative::new(rvid, definition), } } @@ -100,44 +158,72 @@ impl RegionTracker { /// The largest universe this SCC can name. It's the smallest /// largest nameable uninverse of any reachable region. pub(crate) fn max_nameable_universe(self) -> UniverseIndex { - self.max_nameable_universe - } - - fn merge_min_max_seen(&mut self, other: &Self) { - self.max_placeholder_universe_reached = std::cmp::max( - self.max_placeholder_universe_reached, - other.max_placeholder_universe_reached, - ); - - self.max_nameable_universe = - std::cmp::min(self.max_nameable_universe, other.max_nameable_universe); - } - - /// Returns `true` if during the annotated SCC reaches a placeholder - /// with a universe larger than the smallest nameable universe of any - /// reachable existential region. - pub(crate) fn has_incompatible_universes(&self) -> bool { - self.max_nameable_universe().cannot_name(self.max_placeholder_universe_reached) + self.max_nameable_universe.0 } /// Determine if the tracked universes of the two SCCs are compatible. pub(crate) fn universe_compatible_with(&self, other: Self) -> bool { self.max_nameable_universe().can_name(other.max_nameable_universe()) - || self.max_nameable_universe().can_name(other.max_placeholder_universe_reached) + || other.reachable_placeholders.can_be_named_by(self.max_nameable_universe()) + } + + /// If this SCC reaches a placeholder it can't name, return it. + fn unnameable_placeholder(&self) -> Option<(RegionVid, UniverseIndex)> { + let PlaceholderReachability::Placeholders { max_universe: (max_u, max_u_rvid), .. } = + self.reachable_placeholders + else { + return None; + }; + + if self.max_nameable_universe().can_name(max_u) { + return None; + } + + Some((max_u_rvid, max_u)) } } +/// Pick the smallest universe index out of two, preferring +/// the first argument if they are equal. +#[inline(always)] +fn pick_min_max_universe(a: RegionTracker, b: RegionTracker) -> (UniverseIndex, RegionVid) { + std::cmp::min_by_key( + a.max_nameable_universe, + b.max_nameable_universe, + |x: &(UniverseIndex, RegionVid)| x.0, + ) +} impl scc::Annotation for RegionTracker { - fn merge_scc(mut self, other: Self) -> Self { - self.representative = self.representative.merge_scc(other.representative); - self.merge_min_max_seen(&other); - self + fn merge_scc(self, other: Self) -> Self { + trace!("{:?} << {:?}", self.representative, other.representative); + + Self { + reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders), + max_nameable_universe: pick_min_max_universe(self, other), + representative: self.representative.merge_scc(other.representative), + } } fn merge_reached(mut self, other: Self) -> Self { - // No update to in-component values, only add seen values. - self.merge_min_max_seen(&other); - self + let already_has_unnameable_placeholder = self.unnameable_placeholder().is_some(); + self.max_nameable_universe = pick_min_max_universe(self, other); + // This detail is subtle. We stop early here, because there may be multiple + // illegally reached regions, but they are not equally good as blame candidates. + // In general, the ones with the smallest indices of their RegionVids will + // be the best ones, and those will also be visited first. This code + // then will suptly prefer a universe violation happening close from where the + // constraint graph walk started over one that happens later. + // FIXME: a potential optimisation if this is slow is to reimplement + // this check as a boolean fuse, since it will idempotently turn + // true once triggered and never go false again. + if already_has_unnameable_placeholder { + debug!("SCC already has an unnameable placeholder; no use looking for more!"); + self + } else { + self.reachable_placeholders = + self.reachable_placeholders.merge(other.reachable_placeholders); + self + } } } @@ -270,6 +356,7 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( &scc_annotations, fr_static, &mut outlives_constraints, + &definitions, ); let (constraint_sccs, scc_annotations) = if added_constraints { @@ -304,12 +391,14 @@ fn rewrite_placeholder_outlives<'tcx>( annotations: &SccAnnotations<'_, '_, RegionTracker>, fr_static: RegionVid, outlives_constraints: &mut OutlivesConstraintSet<'tcx>, + definitions: &IndexVec>, ) -> bool { // Changed to `true` if we added any constraints and need to // recompute SCCs. let mut added_constraints = false; let annotations = &annotations.scc_to_annotation; + let constraint_graph: OnceCell> = OnceCell::new(); for scc in sccs.all_sccs() { // No point in adding 'static: 'static! @@ -321,28 +410,103 @@ fn rewrite_placeholder_outlives<'tcx>( let annotation = annotations[scc]; - // If this SCC participates in a universe violation, - // e.g. if it reaches a region with a universe smaller than - // the largest region reached, add a requirement that it must - // outlive `'static`. - if annotation.has_incompatible_universes() { - // Optimisation opportunity: this will add more constraints than - // needed for correctness, since an SCC upstream of another with - // a universe violation will "infect" its downstream SCCs to also - // outlive static. - let scc_representative_outlives_static = OutlivesConstraint { - sup: annotation.representative.rvid(), - sub: fr_static, - category: ConstraintCategory::IllegalUniverse, - locations: Locations::All(rustc_span::DUMMY_SP), - span: rustc_span::DUMMY_SP, - variance_info: VarianceDiagInfo::None, - from_closure: false, - }; - outlives_constraints.push(scc_representative_outlives_static); - added_constraints = true; - debug!("Added {:?}: 'static!", annotation.representative.rvid()); - } + let Some((max_u_rvid, max_u)) = annotation.unnameable_placeholder() else { + continue; + }; + + debug!( + "Placeholder universe {max_u:?} is too large for its SCC, represented by {:?}", + annotation.representative + ); + // We only add one `r: 'static` constraint per SCC, where `r` is the SCC representative. + // That constraint is annotated with some outlives relation `tries: unnameable` where + // `unnameable` is unnameable from `tries` and there is a path in the constraint + // graph between them. + // + // We prefer the representative as `tries` in all cases but one: where the problem + // is that the SCC has had its universe lowered to accomodate some other region and + // no longer can name its representative. In that case, we blame `r: low_u`, where `low_u` + // cannot name `r` so that any explanation always starts with the SCC representative. + let blame_to = if annotation.representative.rvid() == max_u_rvid { + // The SCC's representative is not nameable from some region + // that ends up in the SCC. + let small_universed_rvid = find_region( + outlives_constraints, + constraint_graph.get_or_init(|| outlives_constraints.graph(definitions.len())), + definitions, + max_u_rvid, + |r: RegionVid| definitions[r].universe == annotation.max_nameable_universe(), + fr_static, + ); + debug!( + "{small_universed_rvid:?} lowered our universe to {:?}", + annotation.max_nameable_universe() + ); + small_universed_rvid + } else { + // `max_u_rvid` is not nameable by the SCC's representative. + max_u_rvid + }; + + // FIXME: if we can extract a useful blame span here, future error + // reporting and constraint search can be simplified. + + added_constraints = true; + outlives_constraints.push(OutlivesConstraint { + sup: annotation.representative.rvid(), + sub: fr_static, + category: ConstraintCategory::IllegalPlaceholder( + annotation.representative.rvid(), + blame_to, + ), + locations: Locations::All(rustc_span::DUMMY_SP), + span: rustc_span::DUMMY_SP, + variance_info: VarianceDiagInfo::None, + from_closure: false, + }); } added_constraints } + +// FIXME this is at least partially duplicated code to the constraint search in `region_infer`. +/// Find a region matching a predicate in a set of constraints, using BFS. +fn find_region<'tcx>( + constraints: &OutlivesConstraintSet<'tcx>, + graph: &ConstraintGraph, + definitions: &IndexVec>, + start_region: RegionVid, + target_test: impl Fn(RegionVid) -> bool, + fr_static: RegionVid, +) -> RegionVid { + #[derive(Clone, PartialEq, Eq, Debug)] + enum Trace { + StartRegion, + NotVisited, + Visited, + } + + let graph = RegionGraph::new(constraints, graph, fr_static); + + let mut context = IndexVec::from_elem(Trace::NotVisited, definitions); + context[start_region] = Trace::StartRegion; + + let mut deque = VecDeque::new(); + deque.push_back(start_region); + + while let Some(r) = deque.pop_front() { + if target_test(r) { + return r; + } + + for sub_region in graph.outgoing_regions(r) { + if let Trace::NotVisited = context[sub_region] { + context[sub_region] = Trace::Visited; + deque.push_back(sub_region); + } + } + } + // since this function is used exclusively in this module, we know + // we are only searching for regions we found in the region graph, + // so if we don't find what we are looking for there's a bug somwehere. + bug!("Should have found something!"); +} diff --git a/compiler/rustc_borrowck/src/region_infer/graphviz.rs b/compiler/rustc_borrowck/src/region_infer/graphviz.rs index 1936752b63c6e..d5f4dd9c5f50e 100644 --- a/compiler/rustc_borrowck/src/region_infer/graphviz.rs +++ b/compiler/rustc_borrowck/src/region_infer/graphviz.rs @@ -12,6 +12,9 @@ use rustc_middle::ty::UniverseIndex; use super::*; fn render_outlives_constraint(constraint: &OutlivesConstraint<'_>) -> String { + if let ConstraintCategory::IllegalPlaceholder(from, to) = constraint.category { + return format!("b/c {from:?}: {to:?}"); + } match constraint.locations { Locations::All(_) => "All(...)".to_string(), Locations::Single(loc) => format!("{loc:?}"), diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 5f1b655c6b60d..4f9a017998afc 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -1487,11 +1487,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { { debug!("try_propagate_universal_region_error: fr_minus={:?}", fr_minus); - let blame_span_category = self.find_outlives_blame_span( - longer_fr, - NllRegionVariableOrigin::FreeRegion, - shorter_fr, - ); + let blame_constraint = self + .best_blame_constraint( + longer_fr, + NllRegionVariableOrigin::FreeRegion, + shorter_fr, + ) + .0; // Grow `shorter_fr` until we find some non-local regions. (We // always will.) We'll call them `shorter_fr+` -- they're ever @@ -1507,8 +1509,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { propagated_outlives_requirements.push(ClosureOutlivesRequirement { subject: ClosureOutlivesSubject::Region(fr_minus), outlived_free_region: fr, - blame_span: blame_span_category.1.span, - category: blame_span_category.0, + blame_span: blame_constraint.cause.span, + category: blame_constraint.category, }); } return RegionRelationCheckResult::Propagated; @@ -1585,66 +1587,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - /// We have a constraint `fr1: fr2` that is not satisfied, where - /// `fr2` represents some universal region. Here, `r` is some - /// region where we know that `fr1: r` and this function has the - /// job of determining whether `r` is "to blame" for the fact that - /// `fr1: fr2` is required. - /// - /// This is true under two conditions: - /// - /// - `r == fr2` - /// - `fr2` is `'static` and `r` is some placeholder in a universe - /// that cannot be named by `fr1`; in that case, we will require - /// that `fr1: 'static` because it is the only way to `fr1: r` to - /// be satisfied. (See `add_incompatible_universe`.) - pub(crate) fn provides_universal_region( - &self, - r: RegionVid, - fr1: RegionVid, - fr2: RegionVid, - ) -> bool { - debug!("provides_universal_region(r={:?}, fr1={:?}, fr2={:?})", r, fr1, fr2); - let result = { - r == fr2 || { - fr2 == self.universal_regions().fr_static && self.cannot_name_placeholder(fr1, r) - } - }; - debug!("provides_universal_region: result = {:?}", result); - result - } - - /// If `r2` represents a placeholder region, then this returns - /// `true` if `r1` cannot name that placeholder in its - /// value; otherwise, returns `false`. - pub(crate) fn cannot_name_placeholder(&self, r1: RegionVid, r2: RegionVid) -> bool { - match self.definitions[r2].origin { - NllRegionVariableOrigin::Placeholder(placeholder) => { - let r1_universe = self.definitions[r1].universe; - debug!( - "cannot_name_value_of: universe1={r1_universe:?} placeholder={:?}", - placeholder - ); - r1_universe.cannot_name(placeholder.universe) - } - - NllRegionVariableOrigin::FreeRegion | NllRegionVariableOrigin::Existential { .. } => { - false - } - } - } - - /// Finds a good `ObligationCause` to blame for the fact that `fr1` outlives `fr2`. - pub(crate) fn find_outlives_blame_span( + pub(crate) fn constraint_path_between_regions( &self, - fr1: RegionVid, - fr1_origin: NllRegionVariableOrigin, - fr2: RegionVid, - ) -> (ConstraintCategory<'tcx>, ObligationCause<'tcx>) { - let BlameConstraint { category, cause, .. } = self - .best_blame_constraint(fr1, fr1_origin, |r| self.provides_universal_region(r, fr1, fr2)) - .0; - (category, cause) + from_region: RegionVid, + to_region: RegionVid, + ) -> Option>> { + self.constraint_path_to(from_region, |to| to == to_region, true).map(|o| o.0) } /// Walks the graph of constraints (where `'a: 'b` is considered @@ -1655,11 +1603,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// /// Returns: a series of constraints as well as the region `R` /// that passed the target test. + /// If `include_static_outlives_all` is `true`, then the synthetic + /// outlives constraints `'static -> a` for every region `a` are + /// considered in the search, otherwise they are ignored. #[instrument(skip(self, target_test), ret)] - pub(crate) fn find_constraint_paths_between_regions( + pub(crate) fn constraint_path_to( &self, from_region: RegionVid, target_test: impl Fn(RegionVid) -> bool, + include_static_outlives_all: bool, ) -> Option<(Vec>, RegionVid)> { let mut context = IndexVec::from_elem(Trace::NotVisited, &self.definitions); context[from_region] = Trace::StartRegion; @@ -1674,7 +1626,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { while let Some(r) = deque.pop_front() { debug!( - "find_constraint_paths_between_regions: from_region={:?} r={:?} value={}", + "constraint_path_to: from_region={:?} r={:?} value={}", from_region, r, self.region_value_str(r), @@ -1752,7 +1704,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // If this is the `'static` region and the graph's direction is normal, then set up the // Edges iterator to return all regions (#53178). - if r == fr_static && self.constraint_graph.is_normal() { + if r == fr_static && self.constraint_graph.is_normal() && include_static_outlives_all { for sub in self.constraint_graph.outgoing_edges_from_static() { handle_trace(sub, Trace::FromStatic(sub)); } @@ -1760,10 +1712,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { let edges = self.constraint_graph.outgoing_edges_from_graph(r, &self.constraints); // This loop can be hot. for constraint in edges { - if matches!(constraint.category, ConstraintCategory::IllegalUniverse) { - debug!("Ignoring illegal universe constraint: {constraint:?}"); - continue; - } debug_assert_eq!(constraint.sup, r); handle_trace(constraint.sub, Trace::FromGraph(constraint)); } @@ -1782,42 +1730,48 @@ impl<'tcx> RegionInferenceContext<'tcx> { None } + /// Find a path of outlives constraints from `from` to `to`, + /// taking placeholder blame constraints into account, e.g. + /// if there is a relationship where `r1` reaches `r2` and + /// r2 has a larger universe or if r1 and r2 both come from + /// placeholder regions. + /// + /// Returns the path. It panics if there is no such path. + fn path_to_modulo_placeholders( + &self, + from: RegionVid, + to: RegionVid, + ) -> Vec> { + let path = self.constraint_path_between_regions(from, to).unwrap(); + + // If we are looking for a path to 'static, and we are passing + // through a constraint synthesised from an illegal placeholder + // relation, redirect the search to the placeholder to blame. + // FIXME: the performance of this is Not Great, and the logic + // should be folded into the search itself if possible. + for constraint in path.iter() { + let ConstraintCategory::IllegalPlaceholder(cl_fr, culprit) = constraint.category else { + continue; + }; + + debug!("{culprit:?} is the reason {from:?}: 'static!"); + return self.constraint_path_to(cl_fr, |r| r == culprit, false).unwrap().0; + } + + // No funny business; just return the path! + path + } + /// Finds some region R such that `fr1: R` and `R` is live at `location`. #[instrument(skip(self), level = "trace", ret)] pub(crate) fn find_sub_region_live_at(&self, fr1: RegionVid, location: Location) -> RegionVid { trace!(scc = ?self.constraint_sccs.scc(fr1)); trace!(universe = ?self.max_nameable_universe(self.constraint_sccs.scc(fr1))); - self.find_constraint_paths_between_regions(fr1, |r| { + self.constraint_path_to(fr1, |r| { // First look for some `r` such that `fr1: r` and `r` is live at `location` trace!(?r, liveness_constraints=?self.liveness_constraints.pretty_print_live_points(r)); self.liveness_constraints.is_live_at(r, location) - }) - .or_else(|| { - // If we fail to find that, we may find some `r` such that - // `fr1: r` and `r` is a placeholder from some universe - // `fr1` cannot name. This would force `fr1` to be - // `'static`. - self.find_constraint_paths_between_regions(fr1, |r| { - self.cannot_name_placeholder(fr1, r) - }) - }) - .or_else(|| { - // If we fail to find THAT, it may be that `fr1` is a - // placeholder that cannot "fit" into its SCC. In that - // case, there should be some `r` where `fr1: r` and `fr1` is a - // placeholder that `r` cannot name. We can blame that - // edge. - // - // Remember that if `R1: R2`, then the universe of R1 - // must be able to name the universe of R2, because R2 will - // be at least `'empty(Universe(R2))`, and `R1` must be at - // larger than that. - self.find_constraint_paths_between_regions(fr1, |r| { - self.cannot_name_placeholder(r, fr1) - }) - }) - .map(|(_path, r)| r) - .unwrap() + }, true).unwrap().1 } /// Get the region outlived by `longer_fr` and live at `element`. @@ -1861,22 +1815,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// creating a constraint path that forces `R` to outlive /// `from_region`, and then finding the best choices within that /// path to blame. - #[instrument(level = "debug", skip(self, target_test))] + #[instrument(level = "debug", skip(self))] pub(crate) fn best_blame_constraint( &self, from_region: RegionVid, from_region_origin: NllRegionVariableOrigin, - target_test: impl Fn(RegionVid) -> bool, + to_region: RegionVid, ) -> (BlameConstraint<'tcx>, Vec>) { + assert!(from_region != to_region, "Trying to blame a region for itself!"); // Find all paths - let (path, target_region) = self - .find_constraint_paths_between_regions(from_region, target_test) - .or_else(|| { - self.find_constraint_paths_between_regions(from_region, |r| { - self.cannot_name_placeholder(from_region, r) - }) - }) - .unwrap(); + let path = self.path_to_modulo_placeholders(from_region, to_region); debug!( "path={:#?}", path.iter() @@ -1979,7 +1927,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { ConstraintCategory::Cast { unsize_to: Some(unsize_ty), is_implicit_coercion: true, - } if target_region == self.universal_regions().fr_static + } if to_region == self.universal_regions().fr_static // Mirror the note's condition, to minimize how often this diverts blame. && let ty::Adt(_, args) = unsize_ty.kind() && args.iter().any(|arg| arg.as_type().is_some_and(|ty| ty.is_trait())) @@ -2017,7 +1965,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // specific, and are not used for relations that would make sense to blame. ConstraintCategory::BoringNoLocation => 6, // Do not blame internal constraints. - ConstraintCategory::IllegalUniverse => 7, + ConstraintCategory::IllegalPlaceholder(_, _) => 7, ConstraintCategory::Internal => 8, }; @@ -2054,6 +2002,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { path[best_choice] }; + assert!( + !matches!(best_constraint.category, ConstraintCategory::IllegalPlaceholder(_, _)), + "Illegal placeholder constraint blamed; should have redirected to other region relation" + ); + let blame_constraint = BlameConstraint { category: best_constraint.category, from_closure: best_constraint.from_closure, diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 3fc05f2caf2ad..8204f5a5af2eb 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -149,8 +149,17 @@ pub enum ConstraintCategory<'tcx> { /// A constraint that doesn't correspond to anything the user sees. Internal, - /// An internal constraint derived from an illegal universe relation. - IllegalUniverse, + /// An internal constraint derived from an illegal placeholder relation + /// to this region. The arguments are a source -> drain of a path + /// that caused the problem, used when reporting errors. + IllegalPlaceholder( + #[type_foldable(identity)] + #[type_visitable(ignore)] + ty::RegionVid, + #[type_foldable(identity)] + #[type_visitable(ignore)] + ty::RegionVid, + ), } #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] diff --git a/tests/ui/nll/ice-106874.stderr b/tests/ui/nll/ice-106874.stderr index ead4d490a6248..5bf311932d095 100644 --- a/tests/ui/nll/ice-106874.stderr +++ b/tests/ui/nll/ice-106874.stderr @@ -17,6 +17,15 @@ LL | A(B(C::new(D::new(move |st| f(st))))) = note: ...but it actually implements `FnOnce<(&'1 mut V,)>`, for some specific lifetime `'1` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +error: implementation of `FnOnce` is not general enough + --> $DIR/ice-106874.rs:8:7 + | +LL | A(B(C::new(D::new(move |st| f(st))))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&'2 mut V)` must implement `FnOnce<(&'1 mut V,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 mut V,)>`, for some specific lifetime `'2` + error: implementation of `Fn` is not general enough --> $DIR/ice-106874.rs:8:7 | @@ -34,6 +43,7 @@ LL | A(B(C::new(D::new(move |st| f(st))))) | = note: closure with signature `fn(&'2 mut V)` must implement `FnOnce<(&'1 mut V,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 mut V,)>`, for some specific lifetime `'2` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: implementation of `Fn` is not general enough --> $DIR/ice-106874.rs:8:7 @@ -46,43 +56,36 @@ LL | A(B(C::new(D::new(move |st| f(st))))) = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: implementation of `FnOnce` is not general enough - --> $DIR/ice-106874.rs:8:9 + --> $DIR/ice-106874.rs:8:7 | LL | A(B(C::new(D::new(move |st| f(st))))) - | ^^^^^^ implementation of `FnOnce` is not general enough + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | = note: closure with signature `fn(&'2 mut V)` must implement `FnOnce<(&'1 mut V,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 mut V,)>`, for some specific lifetime `'2` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: implementation of `Fn` is not general enough - --> $DIR/ice-106874.rs:8:9 + --> $DIR/ice-106874.rs:8:7 | LL | A(B(C::new(D::new(move |st| f(st))))) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Fn` is not general enough + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Fn` is not general enough | = note: closure with signature `fn(&'2 mut V)` must implement `Fn<(&'1 mut V,)>`, for any lifetime `'1`... = note: ...but it actually implements `Fn<(&'2 mut V,)>`, for some specific lifetime `'2` - -error: implementation of `FnOnce` is not general enough - --> $DIR/ice-106874.rs:8:9 - | -LL | A(B(C::new(D::new(move |st| f(st))))) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough - | - = note: closure with signature `fn(&'2 mut V)` must implement `FnOnce<(&'1 mut V,)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&'2 mut V,)>`, for some specific lifetime `'2` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: higher-ranked subtype error - --> $DIR/ice-106874.rs:8:41 + --> $DIR/ice-106874.rs:8:7 | LL | A(B(C::new(D::new(move |st| f(st))))) - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: higher-ranked subtype error - --> $DIR/ice-106874.rs:8:41 + --> $DIR/ice-106874.rs:8:7 | LL | A(B(C::new(D::new(move |st| f(st))))) - | ^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` diff --git a/tests/ui/nll/missing-universe-cause-issue-114907.stderr b/tests/ui/nll/missing-universe-cause-issue-114907.stderr index 26ad1efec0558..eeafa6f0616ce 100644 --- a/tests/ui/nll/missing-universe-cause-issue-114907.stderr +++ b/tests/ui/nll/missing-universe-cause-issue-114907.stderr @@ -38,16 +38,16 @@ LL | accept(callback); = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: higher-ranked subtype error - --> $DIR/missing-universe-cause-issue-114907.rs:33:21 + --> $DIR/missing-universe-cause-issue-114907.rs:33:5 | LL | accept(callback); - | ^ + | ^^^^^^^^^^^^^^^^ error: higher-ranked subtype error - --> $DIR/missing-universe-cause-issue-114907.rs:33:21 + --> $DIR/missing-universe-cause-issue-114907.rs:33:5 | LL | accept(callback); - | ^ + | ^^^^^^^^^^^^^^^^ | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` From 07c226b020be5295989aee86d3c0ebafdf8e4f87 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Tue, 17 Jun 2025 12:07:21 +0200 Subject: [PATCH 2/7] Move handling of placeholder errors to before region inference --- .../src/diagnostics/bound_region_errors.rs | 72 ++++++------ .../src/diagnostics/region_errors.rs | 73 ++++++++++-- .../rustc_borrowck/src/handle_placeholders.rs | 111 +++++++++++++++++- compiler/rustc_borrowck/src/lib.rs | 31 +++++ compiler/rustc_borrowck/src/nll.rs | 13 +- .../rustc_borrowck/src/region_infer/mod.rs | 21 +++- 6 files changed, 265 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs index 0de4bd67f0ce7..462cc830571af 100644 --- a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs @@ -24,7 +24,6 @@ use rustc_traits::{type_op_ascribe_user_type_with_span, type_op_prove_predicate_ use tracing::{debug, instrument}; use crate::MirBorrowckCtxt; -use crate::region_infer::values::RegionElement; use crate::session_diagnostics::{ HigherRankedErrorCause, HigherRankedLifetimeError, HigherRankedSubtypeError, }; @@ -49,11 +48,12 @@ impl<'tcx> UniverseInfo<'tcx> { UniverseInfo::RelateTys { expected, found } } + /// Report an error where an element erroneously made its way into `placeholder`. pub(crate) fn report_erroneous_element( &self, mbcx: &mut MirBorrowckCtxt<'_, '_, 'tcx>, placeholder: ty::PlaceholderRegion, - error_element: RegionElement, + error_element: Option, cause: ObligationCause<'tcx>, ) { match *self { @@ -145,52 +145,54 @@ pub(crate) trait TypeOpInfo<'tcx> { error_region: Option>, ) -> Option>; - /// Constraints require that `error_element` appear in the - /// values of `placeholder`, but this cannot be proven to - /// hold. Report an error. + /// Turn a placeholder region into a Region with its universe adjusted by + /// the base universe. + fn region_with_adjusted_universe( + &self, + placeholder: ty::PlaceholderRegion, + tcx: TyCtxt<'tcx>, + ) -> ty::Region<'tcx> { + let Some(adjusted_universe) = + placeholder.universe.as_u32().checked_sub(self.base_universe().as_u32()) + else { + unreachable!( + "Could not adjust universe {:?} of {placeholder:?} by base universe {:?}", + placeholder.universe, + self.base_universe() + ); + }; + ty::Region::new_placeholder( + tcx, + ty::Placeholder { universe: adjusted_universe.into(), bound: placeholder.bound }, + ) + } + + /// Report an error where an erroneous element reaches `placeholder`. + /// The erroneous element is either another placeholder that we provide, + /// or we reverse engineer what happened later anyway. #[instrument(level = "debug", skip(self, mbcx))] fn report_erroneous_element( &self, mbcx: &mut MirBorrowckCtxt<'_, '_, 'tcx>, placeholder: ty::PlaceholderRegion, - error_element: RegionElement, + error_element: Option, cause: ObligationCause<'tcx>, ) { let tcx = mbcx.infcx.tcx; - let base_universe = self.base_universe(); - debug!(?base_universe); - - let Some(adjusted_universe) = - placeholder.universe.as_u32().checked_sub(base_universe.as_u32()) - else { - mbcx.buffer_error(self.fallback_error(tcx, cause.span)); - return; - }; - let placeholder_region = ty::Region::new_placeholder( - tcx, - ty::Placeholder { universe: adjusted_universe.into(), bound: placeholder.bound }, - ); - - let error_region = if let RegionElement::PlaceholderRegion(error_placeholder) = - error_element - { - let adjusted_universe = - error_placeholder.universe.as_u32().checked_sub(base_universe.as_u32()); - adjusted_universe.map(|adjusted| { - ty::Region::new_placeholder( - tcx, - ty::Placeholder { universe: adjusted.into(), bound: error_placeholder.bound }, - ) - }) - } else { - None - }; + // FIXME: these adjusted universes are not (always) the same ones as we compute + // earlier. They probably should be, but the logic downstream is complicated, + // and assumes they use whatever this is. + // + // In fact, this function throws away a lot of interesting information that would + // probably allow bypassing lots of logic downstream for a much simpler flow. + let placeholder_region = self.region_with_adjusted_universe(placeholder, tcx); + let error_element = error_element.map(|e| self.region_with_adjusted_universe(e, tcx)); debug!(?placeholder_region); let span = cause.span; - let nice_error = self.nice_error(mbcx, cause, placeholder_region, error_region); + let nice_error = self.nice_error(mbcx, cause, placeholder_region, error_element); debug!(?nice_error); mbcx.buffer_error(nice_error.unwrap_or_else(|| self.fallback_error(tcx, span))); diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index 829247c4d8885..d17bd8ab675df 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -120,8 +120,31 @@ pub(crate) enum RegionErrorKind<'tcx> { member_region: ty::Region<'tcx>, }, - /// Higher-ranked subtyping error. - BoundUniversalRegionError { + /// 'a outlives 'b, and both are placeholders. + PlaceholderOutlivesPlaceholder { + rvid_a: RegionVid, + rvid_b: RegionVid, + origin_a: ty::PlaceholderRegion, + origin_b: ty::PlaceholderRegion, + }, + + /// 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. + PlaceholderOutlivesExistentialThatCannotNameIt { + /// 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, + }, + + /// Higher-ranked subtyping error. A placeholder outlives + /// either a location or a universal region. + PlaceholderOutlivesLocationOrUniversal { /// The placeholder free region. longer_fr: RegionVid, /// The region element that erroneously must be outlived by `longer_fr`. @@ -388,30 +411,56 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { } } - RegionErrorKind::BoundUniversalRegionError { + RegionErrorKind::PlaceholderOutlivesLocationOrUniversal { longer_fr, placeholder, error_element, + } => self.report_erroneous_rvid_reaches_placeholder( + longer_fr, + placeholder, + self.regioncx.region_from_element(longer_fr, &error_element), + ), + RegionErrorKind::PlaceholderOutlivesPlaceholder { + rvid_a, + rvid_b, + origin_a, + origin_b, } => { - let error_vid = self.regioncx.region_from_element(longer_fr, &error_element); + debug!( + "Placeholder mismatch: {rvid_a:?} ({origin_a:?}) reaches {rvid_b:?} ({origin_b:?})" + ); - // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. let cause = self .regioncx .best_blame_constraint( - longer_fr, - NllRegionVariableOrigin::Placeholder(placeholder), - error_vid, + rvid_a, + NllRegionVariableOrigin::Placeholder(origin_a), + rvid_b, ) .0 .cause; - let universe = placeholder.universe; - let universe_info = self.regioncx.universe_info(universe); - - universe_info.report_erroneous_element(self, placeholder, error_element, cause); + // FIXME We may be able to shorten the code path here, and immediately + // report a `RegionResolutionError::UpperBoundUniverseConflict`, but + // that's left for a future refactoring. + self.regioncx.universe_info(origin_a.universe).report_erroneous_element( + self, + origin_a, + Some(origin_b), + cause, + ); } + RegionErrorKind::PlaceholderOutlivesExistentialThatCannotNameIt { + longer_fr, + existental_that_cannot_name_longer, + placeholder, + } => self.report_erroneous_rvid_reaches_placeholder( + longer_fr, + placeholder, + existental_that_cannot_name_longer, + ), + RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => { if is_reported { self.report_region_error( diff --git a/compiler/rustc_borrowck/src/handle_placeholders.rs b/compiler/rustc_borrowck/src/handle_placeholders.rs index fcb7d794f814e..55e64f7b339ac 100644 --- a/compiler/rustc_borrowck/src/handle_placeholders.rs +++ b/compiler/rustc_borrowck/src/handle_placeholders.rs @@ -13,12 +13,12 @@ use rustc_infer::infer::RegionVariableOrigin; use rustc_middle::bug; use rustc_middle::mir::ConstraintCategory; use rustc_middle::ty::{RegionVid, UniverseIndex}; -use tracing::{debug, trace}; +use tracing::{debug, instrument, trace}; use crate::constraints::graph::{ConstraintGraph, Normal, RegionGraph}; use crate::constraints::{ConstraintSccIndex, OutlivesConstraintSet}; use crate::consumers::OutlivesConstraint; -use crate::diagnostics::UniverseInfo; +use crate::diagnostics::{RegionErrorKind, RegionErrors, UniverseInfo}; use crate::member_constraints::MemberConstraintSet; use crate::region_infer::values::{LivenessValues, PlaceholderIndices}; use crate::region_infer::{ConstraintSccs, RegionDefinition, Representative, TypeTest}; @@ -181,6 +181,43 @@ impl RegionTracker { Some((max_u_rvid, max_u)) } + + /// Check for the second and final type of placeholder leak, + /// where a placeholder `'p` outlives (transitively) an existential `'e` + /// and `'e` cannot name `'p`. This is sort of a dual of `unnameable_placeholder`; + /// one of the members of this SCC cannot be named by the SCC. + /// + /// Returns *a* culprit (though there may be more than one). + fn reaches_existential_that_cannot_name_us(&self) -> Option { + let Representative::Placeholder(_p) = self.representative else { + return None; + }; + + let (reachable_lowest_max_u, reachable_lowest_max_u_rvid) = self.max_nameable_universe; + + (!self.reachable_placeholders.can_be_named_by(reachable_lowest_max_u)) + .then_some(reachable_lowest_max_u_rvid) + } + + /// Determine if this SCC reaches a placeholder that isn't `placeholder_rvid`, + /// returning it if that is the case. This prefers the placeholder with the + /// smallest region variable ID. + fn reaches_other_placeholder(&self, placeholder_rvid: RegionVid) -> Option { + match self.reachable_placeholders { + PlaceholderReachability::NoPlaceholders => None, + PlaceholderReachability::Placeholders { min_placeholder, max_placeholder, .. } + if min_placeholder == max_placeholder => + { + None + } + PlaceholderReachability::Placeholders { min_placeholder, max_placeholder, .. } + if min_placeholder == placeholder_rvid => + { + Some(max_placeholder) + } + PlaceholderReachability::Placeholders { min_placeholder, .. } => Some(min_placeholder), + } + } } /// Pick the smallest universe index out of two, preferring /// the first argument if they are equal. @@ -293,6 +330,7 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( constraints: MirTypeckRegionConstraints<'tcx>, universal_region_relations: &Frozen>, infcx: &BorrowckInferCtxt<'tcx>, + errors_buffer: &mut RegionErrors<'tcx>, ) -> LoweredConstraints<'tcx> { let universal_regions = &universal_region_relations.universal_regions; let (definitions, has_placeholders) = region_definitions(universal_regions, infcx); @@ -359,6 +397,13 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( &definitions, ); + find_placeholder_mismatch_errors( + &definitions, + &constraint_sccs, + &scc_annotations, + errors_buffer, + ); + let (constraint_sccs, scc_annotations) = if added_constraints { let mut annotations = SccAnnotations::init(&definitions); @@ -510,3 +555,65 @@ fn find_region<'tcx>( // so if we don't find what we are looking for there's a bug somwehere. bug!("Should have found something!"); } + +/// Identify errors where placeholders illegally reach other regions, and generate +/// errors stored into `errors_buffer`. +/// +/// There are two sources of such errors: +/// 1. A placeholder reaches (possibly transitively) another placeholder. +/// 2. A placeholder `p` reaches (possibly transitively) an existential `e`, +/// where `e` has an allowed maximum universe smaller than `p`'s. +/// +/// There are other potential placeholder errors, but those are detected after +/// region inference, since it may apply type tests or member constraints that +/// alter the contents of SCCs and thus can't be detected at this point. +#[instrument(skip(definitions, sccs, annotations, errors_buffer), level = "debug")] +fn find_placeholder_mismatch_errors<'tcx>( + definitions: &IndexVec>, + sccs: &Sccs, + annotations: &SccAnnotations<'_, '_, RegionTracker>, + errors_buffer: &mut RegionErrors<'tcx>, +) { + use NllRegionVariableOrigin::Placeholder; + for (rvid, definition) in definitions.iter_enumerated() { + let Placeholder(origin_a) = definition.origin else { + continue; + }; + + let scc = sccs.scc(rvid); + let annotation = annotations.scc_to_annotation[scc]; + + if let Some(existental_that_cannot_name_rvid) = + annotation.reaches_existential_that_cannot_name_us() + { + errors_buffer.push(RegionErrorKind::PlaceholderOutlivesExistentialThatCannotNameIt { + 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; + }; + + debug!( + "Placeholder {rvid:?} of SCC {scc:?} reaches other placeholder {other_placeholder:?}" + ); + + // FIXME SURELY there is a neater way to do this? + let Placeholder(origin_b) = definitions[other_placeholder].origin else { + unreachable!( + "Region {rvid:?}, {other_placeholder:?} should be placeholders but aren't!" + ); + }; + + errors_buffer.push(RegionErrorKind::PlaceholderOutlivesPlaceholder { + rvid_a: rvid, + rvid_b: other_placeholder, + origin_a, + origin_b, + }); + } +} diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 4d85f1090201c..6191b331fb9bc 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2625,6 +2625,37 @@ 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, + error_vid: RegionVid, + ) { + // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. + let cause = self + .regioncx + .best_blame_constraint( + longer_fr, + NllRegionVariableOrigin::Placeholder(placeholder), + error_vid, + ) + .0 + .cause; + + // 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, + None, + cause, + ); + } } /// The degree of overlap between 2 places for borrow-checking. diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 1b011d733854c..c6448e2013db3 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -114,10 +114,13 @@ pub(crate) fn compute_regions<'tcx>( Rc::clone(&location_map), ); + let mut placeholder_errors = RegionErrors::new(infcx.tcx); + let lowered_constraints = compute_sccs_applying_placeholder_outlives_constraints( constraints, &universal_region_relations, infcx, + &mut placeholder_errors, ); // If requested, emit legacy polonius facts. @@ -167,9 +170,17 @@ pub(crate) fn compute_regions<'tcx>( }); // Solve the region constraints. - let (closure_region_requirements, nll_errors) = + let (closure_region_requirements, region_inference_errors) = regioncx.solve(infcx, body, polonius_output.clone()); + let nll_errors = if region_inference_errors.has_errors().is_some() { + debug!("Errors already reported, skipping these: {placeholder_errors:?}"); + region_inference_errors + } else { + // Only flag the higher-kinded bounds errors if there are no borrowck errors. + placeholder_errors + }; + if let Some(guar) = nll_errors.has_errors() { // Suppress unhelpful extra errors in `infer_opaque_types`. infcx.set_tainted_by_errors(guar); diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 4f9a017998afc..703076e2b0c3b 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -1531,16 +1531,25 @@ impl<'tcx> RegionInferenceContext<'tcx> { let longer_fr_scc = self.constraint_sccs.scc(longer_fr); debug!("check_bound_universal_region: longer_fr_scc={:?}", longer_fr_scc,); + // FIXME(amandasystems): This is an inlined version of elements_contained_in, without + // placeholders, which are handled separately. Later, when placeholders are removed + // from scc_values, this part will just be elements_contained_in(): + let mut non_placeholder_regions_in = self + .scc_values + .locations_outlived_by(longer_fr_scc) + .map(RegionElement::Location) + .chain( + self.scc_values + .universal_regions_outlived_by(longer_fr_scc) + .map(RegionElement::RootUniversalRegion), + ); + // If we have some bound universal region `'a`, then the only // elements it can contain is itself -- we don't know anything // else about it! - if let Some(error_element) = self - .scc_values - .elements_contained_in(longer_fr_scc) - .find(|e| *e != RegionElement::PlaceholderRegion(placeholder)) - { + if let Some(error_element) = non_placeholder_regions_in.next() { // Stop after the first error, it gets too noisy otherwise, and does not provide more information. - errors_buffer.push(RegionErrorKind::BoundUniversalRegionError { + errors_buffer.push(RegionErrorKind::PlaceholderOutlivesLocationOrUniversal { longer_fr, error_element, placeholder, From 96b5ee831499548a503382b22136b3db218cba48 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Tue, 17 Jun 2025 16:18:07 +0200 Subject: [PATCH 3/7] Bless omitted error messages --- .../bugs/issue-100013.stderr | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tests/ui/generic-associated-types/bugs/issue-100013.stderr b/tests/ui/generic-associated-types/bugs/issue-100013.stderr index ff82aebfef911..e2214c56b4cf7 100644 --- a/tests/ui/generic-associated-types/bugs/issue-100013.stderr +++ b/tests/ui/generic-associated-types/bugs/issue-100013.stderr @@ -9,17 +9,6 @@ LL | | } | = note: this is a known limitation that will be removed in the future (see issue #100013 for more information) -error: lifetime bound not satisfied - --> $DIR/issue-100013.rs:22:5 - | -LL | / async { // a coroutine checked for autotrait impl `Send` -LL | | let x = None::>; // a type referencing GAT -LL | | async {}.await; // a yield point -LL | | } - | |_____^ - | - = note: this is a known limitation that will be removed in the future (see issue #100013 for more information) - error: lifetime may not live long enough --> $DIR/issue-100013.rs:23:17 | @@ -44,5 +33,5 @@ LL | | } | = note: this is a known limitation that will be removed in the future (see issue #100013 for more information) -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors From 199f9610fd04eba370432cf0db5faa8d159986d1 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Tue, 17 Jun 2025 16:24:31 +0200 Subject: [PATCH 4/7] Revise the tests for `coroutine/auto-trait-regions` --- tests/ui/coroutine/auto-trait-regions.rs | 24 +++++++++++--------- tests/ui/coroutine/auto-trait-regions.stderr | 16 ++++++------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/tests/ui/coroutine/auto-trait-regions.rs b/tests/ui/coroutine/auto-trait-regions.rs index f115896a473cd..b3a1dbc64ab5a 100644 --- a/tests/ui/coroutine/auto-trait-regions.rs +++ b/tests/ui/coroutine/auto-trait-regions.rs @@ -20,6 +20,19 @@ impl<'a> Foo for &'a OnlyFooIfRef {} fn assert_foo(f: T) {} +fn other_assertion() { + // Disallow impls which relates lifetimes in the coroutine interior + let gen = #[coroutine] move || { + let a = A(&mut true, &mut true, No); + //~^ ERROR borrow may still be in use when coroutine yields + //~| ERROR borrow may still be in use when coroutine yields + yield; + assert_foo(a); + }; + assert_foo(gen); + //~^ ERROR not general enough +} + fn main() { // Make sure 'static is erased for coroutine interiors so we can't match it in trait selection let x: &'static _ = &OnlyFooIfStaticRef(No); @@ -39,15 +52,4 @@ fn main() { assert_foo(x); }; assert_foo(gen); // ok - - // Disallow impls which relates lifetimes in the coroutine interior - let gen = #[coroutine] move || { - let a = A(&mut true, &mut true, No); - //~^ ERROR borrow may still be in use when coroutine yields - //~| ERROR borrow may still be in use when coroutine yields - yield; - assert_foo(a); - }; - assert_foo(gen); - //~^ ERROR not general enough } diff --git a/tests/ui/coroutine/auto-trait-regions.stderr b/tests/ui/coroutine/auto-trait-regions.stderr index 77b5f3ce57c4a..172d1e612aaeb 100644 --- a/tests/ui/coroutine/auto-trait-regions.stderr +++ b/tests/ui/coroutine/auto-trait-regions.stderr @@ -1,5 +1,5 @@ error[E0626]: borrow may still be in use when coroutine yields - --> $DIR/auto-trait-regions.rs:45:19 + --> $DIR/auto-trait-regions.rs:26:19 | LL | let gen = #[coroutine] move || { | ------- within this coroutine @@ -15,7 +15,7 @@ LL | let gen = #[coroutine] static move || { | ++++++ error[E0626]: borrow may still be in use when coroutine yields - --> $DIR/auto-trait-regions.rs:45:30 + --> $DIR/auto-trait-regions.rs:26:30 | LL | let gen = #[coroutine] move || { | ------- within this coroutine @@ -31,22 +31,22 @@ LL | let gen = #[coroutine] static move || { | ++++++ error: implementation of `Foo` is not general enough - --> $DIR/auto-trait-regions.rs:31:5 + --> $DIR/auto-trait-regions.rs:32:5 | LL | assert_foo(gen); | ^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | - = note: `&'0 OnlyFooIfStaticRef` must implement `Foo`, for any lifetime `'0`... - = note: ...but `Foo` is actually implemented for the type `&'static OnlyFooIfStaticRef` + = note: `Foo` would have to be implemented for the type `A<'0, '1>`, for any two lifetimes `'0` and `'1`... + = note: ...but `Foo` is actually implemented for the type `A<'_, '2>`, for some specific lifetime `'2` error: implementation of `Foo` is not general enough - --> $DIR/auto-trait-regions.rs:51:5 + --> $DIR/auto-trait-regions.rs:44:5 | LL | assert_foo(gen); | ^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | - = note: `Foo` would have to be implemented for the type `A<'0, '1>`, for any two lifetimes `'0` and `'1`... - = note: ...but `Foo` is actually implemented for the type `A<'_, '2>`, for some specific lifetime `'2` + = note: `&'0 OnlyFooIfStaticRef` must implement `Foo`, for any lifetime `'0`... + = note: ...but `Foo` is actually implemented for the type `&'static OnlyFooIfStaticRef` error: aborting due to 4 previous errors From 4958ec85d101701b9fcf5090fc234aa417a3ef31 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Mon, 16 Jun 2025 16:08:24 +0200 Subject: [PATCH 5/7] Remove precise placeholder tracking during region inference Placeholders are now handled mostly by rewriting constraints as a pre-computation step, so precisely tracking which placeholder end up in which region is no longer necessary. --- .../src/diagnostics/bound_region_errors.rs | 1 - .../src/diagnostics/region_errors.rs | 63 ++++---- .../rustc_borrowck/src/handle_placeholders.rs | 14 +- .../rustc_borrowck/src/region_infer/mod.rs | 150 +++++------------- .../src/region_infer/opaque_types.rs | 13 +- .../rustc_borrowck/src/region_infer/values.rs | 106 +------------ compiler/rustc_borrowck/src/type_check/mod.rs | 40 ++--- 7 files changed, 97 insertions(+), 290 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs index 462cc830571af..b3f445cb8c83f 100644 --- a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs @@ -179,7 +179,6 @@ pub(crate) trait TypeOpInfo<'tcx> { cause: ObligationCause<'tcx>, ) { let tcx = mbcx.infcx.tcx; - // FIXME: these adjusted universes are not (always) the same ones as we compute // earlier. They probably should be, but the logic downstream is complicated, // and assumes they use whatever this is. diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index d17bd8ab675df..3dff5474f1de2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -240,49 +240,39 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { &self, diag: &mut Diag<'_>, lower_bound: RegionVid, - ) { + ) -> Option<()> { let mut suggestions = vec![]; let tcx = self.infcx.tcx; - // find generic associated types in the given region 'lower_bound' - let gat_id_and_generics = self - .regioncx - .placeholders_contained_in(lower_bound) - .map(|placeholder| { - if let Some(id) = placeholder.bound.kind.get_id() - && let Some(placeholder_id) = id.as_local() - && let gat_hir_id = tcx.local_def_id_to_hir_id(placeholder_id) - && let Some(generics_impl) = - tcx.parent_hir_node(tcx.parent_hir_id(gat_hir_id)).generics() - { - Some((gat_hir_id, generics_impl)) - } else { - None - } - }) - .collect::>(); - debug!(?gat_id_and_generics); - // find higher-ranked trait bounds bounded to the generic associated types + let scc = self.regioncx.constraint_sccs().scc(lower_bound); + + let placeholder: ty::PlaceholderRegion = self.regioncx.placeholder_representative(scc)?; + + let placeholder_id = placeholder.bound.kind.get_id()?.as_local()?; + let gat_hir_id = self.infcx.tcx.local_def_id_to_hir_id(placeholder_id); + let generics_impl = + self.infcx.tcx.parent_hir_node(self.infcx.tcx.parent_hir_id(gat_hir_id)).generics()?; + let mut hrtb_bounds = vec![]; - gat_id_and_generics.iter().flatten().for_each(|(gat_hir_id, generics)| { - for pred in generics.predicates { - let BoundPredicate(WhereBoundPredicate { bound_generic_params, bounds, .. }) = - pred.kind - else { - continue; - }; - if bound_generic_params - .iter() - .rfind(|bgp| tcx.local_def_id_to_hir_id(bgp.def_id) == *gat_hir_id) - .is_some() - { - for bound in *bounds { - hrtb_bounds.push(bound); - } + + for pred in generics_impl.predicates { + let BoundPredicate(WhereBoundPredicate { bound_generic_params, bounds, .. }) = + pred.kind + else { + continue; + }; + if bound_generic_params + .iter() + .rfind(|bgp| self.infcx.tcx.local_def_id_to_hir_id(bgp.def_id) == gat_hir_id) + .is_some() + { + for bound in *bounds { + hrtb_bounds.push(bound); } } - }); + } + debug!(?hrtb_bounds); hrtb_bounds.iter().for_each(|bound| { @@ -327,6 +317,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { Applicability::MaybeIncorrect, ); } + Some(()) } /// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`. diff --git a/compiler/rustc_borrowck/src/handle_placeholders.rs b/compiler/rustc_borrowck/src/handle_placeholders.rs index 55e64f7b339ac..6369a8619d9ae 100644 --- a/compiler/rustc_borrowck/src/handle_placeholders.rs +++ b/compiler/rustc_borrowck/src/handle_placeholders.rs @@ -20,7 +20,7 @@ use crate::constraints::{ConstraintSccIndex, OutlivesConstraintSet}; use crate::consumers::OutlivesConstraint; use crate::diagnostics::{RegionErrorKind, RegionErrors, UniverseInfo}; use crate::member_constraints::MemberConstraintSet; -use crate::region_infer::values::{LivenessValues, PlaceholderIndices}; +use crate::region_infer::values::LivenessValues; use crate::region_infer::{ConstraintSccs, RegionDefinition, Representative, TypeTest}; use crate::ty::VarianceDiagInfo; use crate::type_check::free_region_relations::UniversalRegionRelations; @@ -39,7 +39,6 @@ pub(crate) struct LoweredConstraints<'tcx> { pub(crate) type_tests: Vec>, pub(crate) liveness_constraints: LivenessValues, pub(crate) universe_causes: FxIndexMap>, - pub(crate) placeholder_indices: PlaceholderIndices, } impl<'d, 'tcx, A: scc::Annotation> SccAnnotations<'d, 'tcx, A> { @@ -69,7 +68,7 @@ impl scc::Annotations for SccAnnotations<'_, '_, RegionTracker> { } #[derive(Copy, Debug, Clone, PartialEq, Eq)] -enum PlaceholderReachability { +pub(crate) enum PlaceholderReachability { /// This SCC reaches no placeholders. NoPlaceholders, /// This SCC reaches at least one placeholder. @@ -218,6 +217,10 @@ impl RegionTracker { PlaceholderReachability::Placeholders { min_placeholder, .. } => Some(min_placeholder), } } + + pub(crate) fn reachable_placeholders(&self) -> PlaceholderReachability { + self.reachable_placeholders + } } /// Pick the smallest universe index out of two, preferring /// the first argument if they are equal. @@ -336,13 +339,12 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( let (definitions, has_placeholders) = region_definitions(universal_regions, infcx); let MirTypeckRegionConstraints { - placeholder_indices, - placeholder_index_to_region: _, liveness_constraints, mut outlives_constraints, mut member_constraints, universe_causes, type_tests, + .. } = constraints; if let Some(guar) = universal_regions.tainted_by_errors() { @@ -384,7 +386,6 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( outlives_constraints: Frozen::freeze(outlives_constraints), liveness_constraints, universe_causes, - placeholder_indices, }; } debug!("Placeholders present; activating placeholder handling logic!"); @@ -427,7 +428,6 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( type_tests, liveness_constraints, universe_causes, - placeholder_indices, } } diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 703076e2b0c3b..cd4c583d71af2 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -28,7 +28,7 @@ use crate::constraints::graph::{self, NormalConstraintGraph, RegionGraph}; use crate::constraints::{ConstraintSccIndex, OutlivesConstraint, OutlivesConstraintSet}; use crate::dataflow::BorrowIndex; use crate::diagnostics::{RegionErrorKind, RegionErrors, UniverseInfo}; -use crate::handle_placeholders::{LoweredConstraints, RegionTracker}; +use crate::handle_placeholders::{LoweredConstraints, PlaceholderReachability, RegionTracker}; use crate::member_constraints::{MemberConstraintSet, NllMemberConstraintIndex}; use crate::polonius::LiveLoans; use crate::polonius::legacy::PoloniusOutput; @@ -360,15 +360,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { outlives_constraints, scc_annotations, type_tests, - liveness_constraints, + mut liveness_constraints, universe_causes, - placeholder_indices, member_constraints, } = lowered_constraints; debug!("universal_regions: {:#?}", universal_region_relations.universal_regions); debug!("outlives constraints: {:#?}", outlives_constraints); - debug!("placeholder_indices: {:#?}", placeholder_indices); debug!("type tests: {:#?}", type_tests); let constraint_graph = Frozen::freeze(outlives_constraints.graph(definitions.len())); @@ -377,8 +375,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { sccs_info(infcx, &constraint_sccs); } - let mut scc_values = - RegionValues::new(location_map, universal_regions.len(), placeholder_indices); + let mut scc_values = RegionValues::new(location_map, universal_regions.len()); for region in liveness_constraints.regions() { let scc = constraint_sccs.scc(region); @@ -388,7 +385,22 @@ impl<'tcx> RegionInferenceContext<'tcx> { let member_constraints = Rc::new(member_constraints.into_mapped(|r| constraint_sccs.scc(r))); - let mut result = Self { + for variable in definitions.indices() { + if let NllRegionVariableOrigin::FreeRegion = definitions[variable].origin { + // For each free, universally quantified region X: + + let scc = constraint_sccs.scc(variable); + + // Add all nodes in the CFG to liveness constraints + liveness_constraints.add_all_points(variable); + scc_values.add_all_points(scc); + + // Add `end(X)` into the set for X. + scc_values.add_element(scc, variable); + } + } + + Self { definitions, liveness_constraints, constraints: outlives_constraints, @@ -402,90 +414,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { scc_values, type_tests, universal_region_relations, - }; - - result.init_free_and_bound_regions(); - - result - } - - /// Initializes the region variables for each universally - /// quantified region (lifetime parameter). The first N variables - /// always correspond to the regions appearing in the function - /// signature (both named and anonymous) and where-clauses. This - /// function iterates over those regions and initializes them with - /// minimum values. - /// - /// For example: - /// ``` - /// fn foo<'a, 'b>( /* ... */ ) where 'a: 'b { /* ... */ } - /// ``` - /// would initialize two variables like so: - /// ```ignore (illustrative) - /// R0 = { CFG, R0 } // 'a - /// R1 = { CFG, R0, R1 } // 'b - /// ``` - /// Here, R0 represents `'a`, and it contains (a) the entire CFG - /// and (b) any universally quantified regions that it outlives, - /// which in this case is just itself. R1 (`'b`) in contrast also - /// outlives `'a` and hence contains R0 and R1. - /// - /// This bit of logic also handles invalid universe relations - /// for higher-kinded types. - /// - /// We Walk each SCC `A` and `B` such that `A: B` - /// and ensure that universe(A) can see universe(B). - /// - /// This serves to enforce the 'empty/placeholder' hierarchy - /// (described in more detail on `RegionKind`): - /// - /// ```ignore (illustrative) - /// static -----+ - /// | | - /// empty(U0) placeholder(U1) - /// | / - /// empty(U1) - /// ``` - /// - /// In particular, imagine we have variables R0 in U0 and R1 - /// created in U1, and constraints like this; - /// - /// ```ignore (illustrative) - /// R1: !1 // R1 outlives the placeholder in U1 - /// R1: R0 // R1 outlives R0 - /// ``` - /// - /// Here, we wish for R1 to be `'static`, because it - /// cannot outlive `placeholder(U1)` and `empty(U0)` any other way. - /// - /// Thanks to this loop, what happens is that the `R1: R0` - /// constraint has lowered the universe of `R1` to `U0`, which in turn - /// means that the `R1: !1` constraint here will cause - /// `R1` to become `'static`. - fn init_free_and_bound_regions(&mut self) { - for variable in self.definitions.indices() { - let scc = self.constraint_sccs.scc(variable); - - match self.definitions[variable].origin { - NllRegionVariableOrigin::FreeRegion => { - // For each free, universally quantified region X: - - // Add all nodes in the CFG to liveness constraints - self.liveness_constraints.add_all_points(variable); - self.scc_values.add_all_points(scc); - - // Add `end(X)` into the set for X. - self.scc_values.add_element(scc, variable); - } - - NllRegionVariableOrigin::Placeholder(placeholder) => { - self.scc_values.add_element(scc, placeholder); - } - - NllRegionVariableOrigin::Existential { .. } => { - // For existential, regions, nothing to do. - } - } } } @@ -542,14 +470,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.scc_values.region_value_str(scc) } - pub(crate) fn placeholders_contained_in( - &self, - r: RegionVid, - ) -> impl Iterator { - let scc = self.constraint_sccs.scc(r); - self.scc_values.placeholders_contained_in(scc) - } - /// 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( @@ -901,8 +821,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { // // It doesn't matter *what* universe because the promoted `T` will // always be in the root universe. - if let Some(p) = self.scc_values.placeholders_contained_in(r_scc).next() { - debug!("encountered placeholder in higher universe: {:?}, requiring 'static", p); + if let PlaceholderReachability::Placeholders { min_placeholder, .. } = + self.scc_annotations[r_scc].reachable_placeholders() + { + debug!( + "encountered placeholder in higher universe: {min_placeholder:?}, requiring 'static" + ); + let static_r = self.universal_regions().fr_static; propagated_outlives_requirements.push(ClosureOutlivesRequirement { subject, @@ -1792,14 +1717,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { match *element { RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l), RegionElement::RootUniversalRegion(r) => r, - RegionElement::PlaceholderRegion(error_placeholder) => self - .definitions - .iter_enumerated() - .find_map(|(r, definition)| match definition.origin { - NllRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r), - _ => None, - }) - .unwrap(), } } @@ -2082,6 +1999,21 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.scc_annotations[scc].representative.rvid() } + /// If the representative of an SCC is a placeholder, return + /// its originating `PlaceholderRegion`. + pub(crate) fn placeholder_representative( + &self, + scc: ConstraintSccIndex, + ) -> Option { + if let Representative::Placeholder(r) = self.scc_annotations[scc].representative + && let NllRegionVariableOrigin::Placeholder(p) = self.definitions[r].origin + { + Some(p) + } else { + None + } + } + pub(crate) fn liveness_constraints(&self) -> &LivenessValues { &self.liveness_constraints } diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 6270e6d9a60ee..e88ac3498f880 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -190,17 +190,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { ty::ReVar(vid) => { let scc = self.constraint_sccs.scc(vid); - // Special handling of higher-ranked regions. - if !self.max_nameable_universe(scc).is_root() { - match self.scc_values.placeholders_contained_in(scc).enumerate().last() { - // If the region contains a single placeholder then they're equal. - Some((0, placeholder)) => { - return ty::Region::new_placeholder(tcx, placeholder); - } - - // Fallback: this will produce a cryptic error message. - _ => return region, - } + if let Some(representative) = self.placeholder_representative(scc) { + return ty::Region::new_placeholder(tcx, representative); } // Find something that we can name diff --git a/compiler/rustc_borrowck/src/region_infer/values.rs b/compiler/rustc_borrowck/src/region_infer/values.rs index f1427218cdb02..1d09ef68f5270 100644 --- a/compiler/rustc_borrowck/src/region_infer/values.rs +++ b/compiler/rustc_borrowck/src/region_infer/values.rs @@ -1,24 +1,18 @@ use std::fmt::Debug; use std::rc::Rc; -use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; +use rustc_data_structures::fx::FxHashSet; use rustc_index::Idx; use rustc_index::bit_set::SparseBitMatrix; use rustc_index::interval::{IntervalSet, SparseIntervalMatrix}; use rustc_middle::mir::{BasicBlock, Location}; -use rustc_middle::ty::{self, RegionVid}; +use rustc_middle::ty::RegionVid; use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex}; use tracing::debug; use crate::BorrowIndex; use crate::polonius::LiveLoans; -rustc_index::newtype_index! { - /// A single integer representing a `ty::Placeholder`. - #[debug_format = "PlaceholderIndex({})"] - pub(crate) struct PlaceholderIndex {} -} - /// An individual element in a region value -- the value of a /// particular region variable consists of a set of these elements. #[derive(Debug, Clone, PartialEq)] @@ -29,10 +23,6 @@ pub(crate) enum RegionElement { /// A universally quantified region from the root universe (e.g., /// a lifetime parameter). RootUniversalRegion(RegionVid), - - /// A placeholder (e.g., instantiated from a `for<'a> fn(&'a u32)` - /// type). - PlaceholderRegion(ty::PlaceholderRegion), } /// Records the CFG locations where each region is live. When we initially compute liveness, we use @@ -190,37 +180,6 @@ impl LivenessValues { } } -/// Maps from `ty::PlaceholderRegion` values that are used in the rest of -/// rustc to the internal `PlaceholderIndex` values that are used in -/// NLL. -#[derive(Debug, Default)] -pub(crate) struct PlaceholderIndices { - indices: FxIndexSet, -} - -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() - } - - pub(crate) fn lookup_index(&self, placeholder: ty::PlaceholderRegion) -> PlaceholderIndex { - self.indices.get_index_of(&placeholder).unwrap().into() - } - - pub(crate) fn lookup_placeholder( - &self, - placeholder: PlaceholderIndex, - ) -> ty::PlaceholderRegion { - self.indices[placeholder.index()] - } - - pub(crate) fn len(&self) -> usize { - self.indices.len() - } -} - /// Stores the full values for a set of regions (in contrast to /// `LivenessValues`, which only stores those points in the where a /// region is live). The full value for a region may contain points in @@ -241,32 +200,20 @@ impl PlaceholderIndices { /// it would also contain various points from within the function. pub(crate) struct RegionValues { location_map: Rc, - placeholder_indices: PlaceholderIndices, points: SparseIntervalMatrix, free_regions: SparseBitMatrix, - - /// Placeholders represent bound regions -- so something like `'a` - /// in `for<'a> fn(&'a u32)`. - placeholders: SparseBitMatrix, } impl RegionValues { /// Creates a new set of "region values" that tracks causal information. /// Each of the regions in num_region_variables will be initialized with an /// empty set of points and no causal information. - pub(crate) fn new( - location_map: Rc, - num_universal_regions: usize, - placeholder_indices: PlaceholderIndices, - ) -> Self { + pub(crate) fn new(location_map: Rc, num_universal_regions: usize) -> Self { let num_points = location_map.num_points(); - let num_placeholders = placeholder_indices.len(); Self { location_map, points: SparseIntervalMatrix::new(num_points), - placeholder_indices, free_regions: SparseBitMatrix::new(num_universal_regions), - placeholders: SparseBitMatrix::new(num_placeholders), } } @@ -285,9 +232,7 @@ impl RegionValues { /// Adds all elements in `r_from` to `r_to` (because e.g., `r_to: /// r_from`). pub(crate) fn add_region(&mut self, r_to: N, r_from: N) -> bool { - self.points.union_rows(r_from, r_to) - | self.free_regions.union_rows(r_from, r_to) - | self.placeholders.union_rows(r_from, r_to) + self.points.union_rows(r_from, r_to) | self.free_regions.union_rows(r_from, r_to) } /// Returns `true` if the region `r` contains the given element. @@ -354,28 +299,16 @@ impl RegionValues { } /// Returns all the elements contained in a given region's value. - pub(crate) fn placeholders_contained_in( - &self, + pub(crate) fn elements_contained_in<'a>( + &'a self, r: N, - ) -> impl Iterator { - self.placeholders - .row(r) - .into_iter() - .flat_map(|set| set.iter()) - .map(move |p| self.placeholder_indices.lookup_placeholder(p)) - } - - /// Returns all the elements contained in a given region's value. - pub(crate) fn elements_contained_in(&self, r: N) -> impl Iterator { + ) -> impl Iterator + 'a { let points_iter = self.locations_outlived_by(r).map(RegionElement::Location); let free_regions_iter = self.universal_regions_outlived_by(r).map(RegionElement::RootUniversalRegion); - let placeholder_universes_iter = - self.placeholders_contained_in(r).map(RegionElement::PlaceholderRegion); - - points_iter.chain(free_regions_iter).chain(placeholder_universes_iter) + points_iter.chain(free_regions_iter) } /// Returns a "pretty" string value of the region. Meant for debugging. @@ -412,18 +345,6 @@ impl ToElementIndex for RegionVid { } } -impl ToElementIndex for ty::PlaceholderRegion { - fn add_to_row(self, values: &mut RegionValues, row: N) -> bool { - let index = values.placeholder_indices.lookup_index(self); - values.placeholders.insert(row, index) - } - - fn contained_in_row(self, values: &RegionValues, row: N) -> bool { - let index = values.placeholder_indices.lookup_index(self); - values.placeholders.contains(row, index) - } -} - /// For debugging purposes, returns a pretty-printed string of the given points. pub(crate) fn pretty_print_points( location_map: &DenseLocationMap, @@ -483,17 +404,6 @@ fn pretty_print_region_elements(elements: impl IntoIterator { - if let Some((location1, location2)) = open_location { - push_sep(&mut result); - push_location_range(&mut result, location1, location2); - open_location = None; - } - - push_sep(&mut result); - result.push_str(&format!("{placeholder:?}")); - } } } diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 9b6dcfd17c62e..bf0d7bd212166 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -5,13 +5,13 @@ use std::{fmt, iter, mem}; use rustc_abi::FieldIdx; use rustc_data_structures::frozen::Frozen; -use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::LocalDefId; use rustc_hir::lang_items::LangItem; -use rustc_index::{IndexSlice, IndexVec}; +use rustc_index::IndexSlice; use rustc_infer::infer::canonical::QueryRegionConstraints; use rustc_infer::infer::outlives::env::RegionBoundPairs; use rustc_infer::infer::region_constraints::RegionConstraintData; @@ -46,7 +46,7 @@ use crate::member_constraints::MemberConstraintSet; use crate::polonius::legacy::{PoloniusFacts, PoloniusLocationTable}; use crate::polonius::{PoloniusContext, PoloniusLivenessContext}; use crate::region_infer::TypeTest; -use crate::region_infer::values::{LivenessValues, PlaceholderIndex, PlaceholderIndices}; +use crate::region_infer::values::LivenessValues; use crate::session_diagnostics::{MoveUnsized, SimdIntrinsicArgConst}; use crate::type_check::free_region_relations::{CreateResult, UniversalRegionRelations}; use crate::universal_regions::{DefiningTy, UniversalRegions}; @@ -110,8 +110,7 @@ pub(crate) fn type_check<'tcx>( location_map: Rc, ) -> MirTypeckResults<'tcx> { let mut constraints = MirTypeckRegionConstraints { - placeholder_indices: PlaceholderIndices::default(), - placeholder_index_to_region: IndexVec::default(), + placeholder_to_region: FxHashMap::default(), liveness_constraints: LivenessValues::with_specific_points(Rc::clone(&location_map)), outlives_constraints: OutlivesConstraintSet::default(), member_constraints: MemberConstraintSet::default(), @@ -236,19 +235,10 @@ pub(crate) struct MirTypeckResults<'tcx> { /// A collection of region constraints that must be satisfied for the /// program to be considered well-typed. pub(crate) struct MirTypeckRegionConstraints<'tcx> { - /// Maps from a `ty::Placeholder` to the corresponding - /// `PlaceholderIndex` bit that we will use for it. - /// - /// To keep everything in sync, do not insert this set - /// directly. Instead, use the `placeholder_region` helper. - pub(crate) placeholder_indices: PlaceholderIndices, - - /// Each time we add a placeholder to `placeholder_indices`, we - /// also create a corresponding "representative" region vid for - /// that wraps it. This vector tracks those. This way, when we - /// convert the same `ty::RePlaceholder(p)` twice, we can map to - /// the same underlying `RegionVid`. - pub(crate) placeholder_index_to_region: IndexVec>, + /// For each placeholder we create a corresponding representative region vid. + /// This map tracks those. This way, when we convert the same `ty::RePlaceholder(p)` + /// twice, we can map to the same underlying `RegionVid`. + placeholder_to_region: FxHashMap>, /// In general, the type-checker is not responsible for enforcing /// liveness constraints; this job falls to the region inferencer, @@ -276,16 +266,10 @@ impl<'tcx> MirTypeckRegionConstraints<'tcx> { infcx: &InferCtxt<'tcx>, placeholder: ty::PlaceholderRegion, ) -> ty::Region<'tcx> { - let placeholder_index = self.placeholder_indices.insert(placeholder); - match self.placeholder_index_to_region.get(placeholder_index) { - Some(&v) => v, - None => { - let origin = NllRegionVariableOrigin::Placeholder(placeholder); - let region = infcx.next_nll_region_var_in_universe(origin, placeholder.universe); - self.placeholder_index_to_region.push(region); - region - } - } + *self.placeholder_to_region.entry(placeholder).or_insert_with(|| { + let origin = NllRegionVariableOrigin::Placeholder(placeholder); + infcx.next_nll_region_var_in_universe(origin, placeholder.universe) + }) } } From 3212751145bd7e001409092131107aac6d30e3d3 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Wed, 25 Jun 2025 15:27:21 +0200 Subject: [PATCH 6/7] Move the placeholder-to-region vid mapping out of `MirTypckRegionConstraints` --- .../rustc_borrowck/src/handle_placeholders.rs | 1 - .../src/type_check/constraint_conversion.rs | 9 ++- .../src/type_check/free_region_relations.rs | 8 ++- compiler/rustc_borrowck/src/type_check/mod.rs | 62 ++++++++++++------- .../src/type_check/relate_tys.rs | 6 +- 5 files changed, 56 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_borrowck/src/handle_placeholders.rs b/compiler/rustc_borrowck/src/handle_placeholders.rs index 6369a8619d9ae..8ab0378fbda3f 100644 --- a/compiler/rustc_borrowck/src/handle_placeholders.rs +++ b/compiler/rustc_borrowck/src/handle_placeholders.rs @@ -344,7 +344,6 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>( mut member_constraints, universe_causes, type_tests, - .. } = constraints; if let Some(guar) = universal_regions.tainted_by_errors() { diff --git a/compiler/rustc_borrowck/src/type_check/constraint_conversion.rs b/compiler/rustc_borrowck/src/type_check/constraint_conversion.rs index 0a114467f4325..fc569c8081bab 100644 --- a/compiler/rustc_borrowck/src/type_check/constraint_conversion.rs +++ b/compiler/rustc_borrowck/src/type_check/constraint_conversion.rs @@ -15,7 +15,7 @@ use tracing::{debug, instrument}; use crate::constraints::OutlivesConstraint; use crate::region_infer::TypeTest; -use crate::type_check::{Locations, MirTypeckRegionConstraints}; +use crate::type_check::{Locations, MirTypeckRegionConstraints, PlaceholderToRegion}; use crate::universal_regions::UniversalRegions; use crate::{ClosureOutlivesSubject, ClosureRegionRequirements, ConstraintCategory}; @@ -40,6 +40,7 @@ pub(crate) struct ConstraintConversion<'a, 'tcx> { category: ConstraintCategory<'tcx>, from_closure: bool, constraints: &'a mut MirTypeckRegionConstraints<'tcx>, + placeholder_to_region: &'a mut PlaceholderToRegion<'tcx>, } impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> { @@ -53,6 +54,7 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> { span: Span, category: ConstraintCategory<'tcx>, constraints: &'a mut MirTypeckRegionConstraints<'tcx>, + placeholder_to_region: &'a mut PlaceholderToRegion<'tcx>, ) -> Self { Self { infcx, @@ -65,6 +67,7 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> { category, constraints, from_closure: false, + placeholder_to_region, } } @@ -206,7 +209,7 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> { if value.has_placeholders() { fold_regions(self.infcx.tcx, value, |r, _| match r.kind() { ty::RePlaceholder(placeholder) => { - self.constraints.placeholder_region(self.infcx, placeholder) + self.placeholder_to_region.placeholder_region(self.infcx, placeholder) } _ => r, }) @@ -227,7 +230,7 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> { fn to_region_vid(&mut self, r: ty::Region<'tcx>) -> ty::RegionVid { if let ty::RePlaceholder(placeholder) = r.kind() { - self.constraints.placeholder_region(self.infcx, placeholder).as_var() + self.placeholder_to_region.placeholder_region(self.infcx, placeholder).as_var() } else { self.universal_regions.to_region_vid(r) } 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 f642d34ea6735..745d2a093559c 100644 --- a/compiler/rustc_borrowck/src/type_check/free_region_relations.rs +++ b/compiler/rustc_borrowck/src/type_check/free_region_relations.rs @@ -14,7 +14,9 @@ use rustc_trait_selection::traits::query::type_op::{self, TypeOp}; use tracing::{debug, instrument}; use type_op::TypeOpOutput; -use crate::type_check::{Locations, MirTypeckRegionConstraints, constraint_conversion}; +use crate::type_check::{ + Locations, MirTypeckRegionConstraints, PlaceholderToRegion, constraint_conversion, +}; use crate::universal_regions::UniversalRegions; #[derive(Debug)] @@ -51,6 +53,7 @@ pub(crate) fn create<'tcx>( param_env: ty::ParamEnv<'tcx>, universal_regions: UniversalRegions<'tcx>, constraints: &mut MirTypeckRegionConstraints<'tcx>, + placeholder_to_region: &mut PlaceholderToRegion<'tcx>, ) -> CreateResult<'tcx> { UniversalRegionRelationsBuilder { infcx, @@ -60,6 +63,7 @@ pub(crate) fn create<'tcx>( region_bound_pairs: Default::default(), outlives: Default::default(), inverse_outlives: Default::default(), + placeholder_to_region, } .create() } @@ -181,6 +185,7 @@ struct UniversalRegionRelationsBuilder<'a, 'tcx> { param_env: ty::ParamEnv<'tcx>, universal_regions: UniversalRegions<'tcx>, constraints: &'a mut MirTypeckRegionConstraints<'tcx>, + placeholder_to_region: &'a mut PlaceholderToRegion<'tcx>, // outputs: outlives: TransitiveRelationBuilder, @@ -324,6 +329,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> { span, ConstraintCategory::Internal, self.constraints, + self.placeholder_to_region, ) .convert_all(c); } diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index bf0d7bd212166..5fb30a6c54760 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -110,7 +110,6 @@ pub(crate) fn type_check<'tcx>( location_map: Rc, ) -> MirTypeckResults<'tcx> { let mut constraints = MirTypeckRegionConstraints { - placeholder_to_region: FxHashMap::default(), liveness_constraints: LivenessValues::with_specific_points(Rc::clone(&location_map)), outlives_constraints: OutlivesConstraintSet::default(), member_constraints: MemberConstraintSet::default(), @@ -118,12 +117,24 @@ pub(crate) fn type_check<'tcx>( universe_causes: FxIndexMap::default(), }; + // FIXME: I strongly suspect this follows the case of being mutated for a while + // and then settling, but I don't know enough about the type inference parts to know + // when this happens and if this can be exploited to simplify some of the downstream + // code. -- @amandasystems. + let mut placeholder_to_region = Default::default(); + let CreateResult { universal_region_relations, region_bound_pairs, normalized_inputs_and_output, known_type_outlives_obligations, - } = free_region_relations::create(infcx, infcx.param_env, universal_regions, &mut constraints); + } = free_region_relations::create( + infcx, + infcx.param_env, + universal_regions, + &mut constraints, + &mut placeholder_to_region, + ); let pre_obligations = infcx.take_registered_region_obligations(); assert!( @@ -155,6 +166,7 @@ pub(crate) fn type_check<'tcx>( borrow_set, constraints: &mut constraints, polonius_liveness, + placeholder_to_region, }; typeck.check_user_type_annotations(); @@ -221,6 +233,7 @@ struct TypeChecker<'a, 'tcx> { constraints: &'a mut MirTypeckRegionConstraints<'tcx>, /// When using `-Zpolonius=next`, the liveness helper data used to create polonius constraints. polonius_liveness: Option, + placeholder_to_region: PlaceholderToRegion<'tcx>, } /// Holder struct for passing results from MIR typeck to the rest of the non-lexical regions @@ -232,14 +245,30 @@ pub(crate) struct MirTypeckResults<'tcx> { pub(crate) polonius_context: Option, } +/// For each placeholder we create a corresponding representative region vid. +/// This map tracks those. This way, when we convert the same `ty::RePlaceholder(p)` +/// twice, we can map to the same underlying `RegionVid`. +#[derive(Default)] +pub(crate) struct PlaceholderToRegion<'tcx>(FxHashMap>); + +impl<'tcx> PlaceholderToRegion<'tcx> { + /// Creates a `Region` for a given `PlaceholderRegion`, or returns the + /// region that corresponds to a previously created one. + fn placeholder_region( + &mut self, + infcx: &InferCtxt<'tcx>, + placeholder: ty::PlaceholderRegion, + ) -> ty::Region<'tcx> { + *self.0.entry(placeholder).or_insert_with(|| { + let origin = NllRegionVariableOrigin::Placeholder(placeholder); + infcx.next_nll_region_var_in_universe(origin, placeholder.universe) + }) + } +} + /// A collection of region constraints that must be satisfied for the /// program to be considered well-typed. pub(crate) struct MirTypeckRegionConstraints<'tcx> { - /// For each placeholder we create a corresponding representative region vid. - /// This map tracks those. This way, when we convert the same `ty::RePlaceholder(p)` - /// twice, we can map to the same underlying `RegionVid`. - placeholder_to_region: FxHashMap>, - /// In general, the type-checker is not responsible for enforcing /// liveness constraints; this job falls to the region inferencer, /// which performs a liveness analysis. However, in some limited @@ -258,21 +287,6 @@ pub(crate) struct MirTypeckRegionConstraints<'tcx> { pub(crate) type_tests: Vec>, } -impl<'tcx> MirTypeckRegionConstraints<'tcx> { - /// Creates a `Region` for a given `PlaceholderRegion`, or returns the - /// region that corresponds to a previously created one. - fn placeholder_region( - &mut self, - infcx: &InferCtxt<'tcx>, - placeholder: ty::PlaceholderRegion, - ) -> ty::Region<'tcx> { - *self.placeholder_to_region.entry(placeholder).or_insert_with(|| { - let origin = NllRegionVariableOrigin::Placeholder(placeholder); - infcx.next_nll_region_var_in_universe(origin, placeholder.universe) - }) - } -} - /// The `Locations` type summarizes *where* region constraints are /// required to hold. Normally, this is at a particular point which /// created the obligation, but for constraints that the user gave, we @@ -350,7 +364,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { fn to_region_vid(&mut self, r: ty::Region<'tcx>) -> RegionVid { if let ty::RePlaceholder(placeholder) = r.kind() { - self.constraints.placeholder_region(self.infcx, placeholder).as_var() + self.placeholder_to_region.placeholder_region(self.infcx, placeholder).as_var() } else { self.universal_regions.to_region_vid(r) } @@ -398,6 +412,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { locations.span(self.body), category, self.constraints, + &mut self.placeholder_to_region, ) .convert_all(data); } @@ -2487,6 +2502,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { self.body.span, // irrelevant; will be overridden. ConstraintCategory::Boring, // same as above. self.constraints, + &mut self.placeholder_to_region, ) .apply_closure_requirements(closure_requirements, def_id, args); } diff --git a/compiler/rustc_borrowck/src/type_check/relate_tys.rs b/compiler/rustc_borrowck/src/type_check/relate_tys.rs index 02a41469c97f4..77ea0388fe02e 100644 --- a/compiler/rustc_borrowck/src/type_check/relate_tys.rs +++ b/compiler/rustc_borrowck/src/type_check/relate_tys.rs @@ -258,8 +258,10 @@ impl<'a, 'b, 'tcx> NllTypeRelating<'a, 'b, 'tcx> { #[instrument(skip(self), level = "debug")] fn next_placeholder_region(&mut self, placeholder: ty::PlaceholderRegion) -> ty::Region<'tcx> { - let reg = - self.type_checker.constraints.placeholder_region(self.type_checker.infcx, placeholder); + let reg = self + .type_checker + .placeholder_to_region + .placeholder_region(self.type_checker.infcx, placeholder); let reg_info = match placeholder.bound.kind { ty::BoundRegionKind::Anon => sym::anon, From 23e3ffabb34af276c42b1e812e5da9b798dd356c Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Wed, 25 Jun 2025 16:00:59 +0200 Subject: [PATCH 7/7] Simplify now that placeholders are gone --- .../rustc_borrowck/src/region_infer/mod.rs | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index cd4c583d71af2..ebbbff8a39b0a 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -1451,29 +1451,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { placeholder: ty::PlaceholderRegion, errors_buffer: &mut RegionErrors<'tcx>, ) { - debug!("check_bound_universal_region(fr={:?}, placeholder={:?})", longer_fr, placeholder,); - let longer_fr_scc = self.constraint_sccs.scc(longer_fr); - debug!("check_bound_universal_region: longer_fr_scc={:?}", longer_fr_scc,); - - // FIXME(amandasystems): This is an inlined version of elements_contained_in, without - // placeholders, which are handled separately. Later, when placeholders are removed - // from scc_values, this part will just be elements_contained_in(): - let mut non_placeholder_regions_in = self - .scc_values - .locations_outlived_by(longer_fr_scc) - .map(RegionElement::Location) - .chain( - self.scc_values - .universal_regions_outlived_by(longer_fr_scc) - .map(RegionElement::RootUniversalRegion), - ); - // If we have some bound universal region `'a`, then the only - // elements it can contain is itself -- we don't know anything - // else about it! - if let Some(error_element) = non_placeholder_regions_in.next() { - // Stop after the first error, it gets too noisy otherwise, and does not provide more information. + debug!( + "check_bound_universal_region(fr={:?}, placeholder={:?}, scc={:?})", + longer_fr, placeholder, longer_fr_scc + ); + + // If we have some bound universal region `'a`, then the only elements it can contain + // is itself -- we don't know anything else about it! + // + // Stop after the first error, it gets too noisy otherwise, and does not provide more information. + if let Some(error_element) = self.scc_values.elements_contained_in(longer_fr_scc).next() { errors_buffer.push(RegionErrorKind::PlaceholderOutlivesLocationOrUniversal { longer_fr, error_element,