From 451912262f69731d6481394b197bb284b8c07b33 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Thu, 5 Jun 2025 13:04:31 +0200 Subject: [PATCH 1/4] 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/4] 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/4] 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/4] 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