From 335fa613415894a49d74b6e0f983b7fd8280bdc0 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Thu, 5 Jun 2025 13:04:31 +0200 Subject: [PATCH] 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 095c0df98acc1..0d4ded1c7bb6a 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -566,7 +566,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`