Skip to content

Commit

Permalink
Auto merge of #130227 - amandasystems:remove-placeholders-completely,…
Browse files Browse the repository at this point in the history 
… r=<try>

[WIP] Remove placeholders completely

This PR does shotgun surgery on borrowck to remove all special handling of placeholders, completely replacing them with a preprocessing step that rewrites placeholder leaks into constraints, removing constraint propagation of placeholders and most of the logic used to detect placeholder violations during error reporting. This finishes what #123720 started. Due to the already sprawling scope of this PR, not all the breaks are clean. In particular, some of the error reporting code can almost certainly be further simplified.

The new method works like this:
1. during SCC construction, some information about SCC membership and reachability is retained
2. just after SCC construction, a constraint `r - (from: to_invalid) - > 'static` is added when `r` is the representative of an SCC and
   1. that SCC either has had its universe shrunk because it reaches a region with a smaller one (in which case `to_invalid` is the smallest-universed region it reaches), or if it reaches a region with a too large universe that isn't part of the SCC (in which case `to_invalid` is the region with a too large universe). In either case, `from` is also `r`.
 2.  some region `reaches` in `r`'s SCC reaches another placeholder, `reached`, in which case the added constraint is `r -> (reaches: reached) 'static`. Through clever choice of defaults (chosing minimum elements), `reached` will be `r` if at all possible.

When tracing errors for diagnostics one of these special constraints along a path are treated much like a HTTP redirect: if we are explaining `from: to` and reach an edge with `reaches: invalid` we stop the search and start following `reaches: invalid` instead. When doing this the implicit edges `x: 'static` for every region `x` are ignored, since the search would otherwise be able to cheat by going through 'static and re-find the same edge again.

Type-tests are also rewritten to account for placeholder issues. In particular, if a bound implies `: 'static`, this is flagged using a new variant, and if a test is guaranteed to always fail (e.g. if an equals bound reaches different placeholders), it is replaced with a bound that is always unsatisfied.

A bunch of optimisations are possible:
- ~~Conservatively add constraints, e.g. one per SCC. May worsen error tracing!~~
- ~~as a final pass, allow fusing the annotations for the SCC after adding the extra constraints to remove unnecessary information and save memory. This could be done cheaply since we already iterate over the entire set of SCCs.~~
- currently, if constraints are added the entire set of SCCs are recomputed. This is of course rather wasteful, and we could do better. Especially since SCCs are added in dependency order. This would require a fully separate SCC module since the dynamic SCC combo we'd need now shares almost no properties with regular SCC computation. Given that this is meant to be a temporary work-around, that seems like too much work.

There are a bunch of rather nice bonuses:
- We now don't need to expose region indices in `MirTypeckRegionConstraints` to the entire crate. The only entry point is `placeholder_region()` so correctness of the indices is now guaranteed
- A lot of things that were previously iterations over lists is now a single lookup
- The constraint graph search functions are simple and at least one of them can now take a proper region as target rather than a predicate function. The only case that needs the predicate argument  to `find_constraint_path_to()` is `find_sub_region_live_at()`, which may or may not be possible to work around.

 r​? nikomatsakis
  • Loading branch information
@bors
bors committed Feb 28, 2025
2 parents 2f58193 + f6d7c6f commit a87ebda
Show file tree
Hide file tree
Showing 26 changed files with 1,846 additions and 1,060 deletions.
20 changes: 15 additions & 5 deletions compiler/rustc_borrowck/src/constraints/graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,17 @@ impl<D: ConstraintGraphDirection> ConstraintGraph<D> {
}

/// Given a region `R`, iterate over all constraints `R: R1`.
/// if `static_region` is `None`, do not yield implicit
/// `'static -> a` edges.
pub(crate) fn outgoing_edges<'a, 'tcx>(
&'a self,
region_sup: RegionVid,
constraints: &'a OutlivesConstraintSet<'tcx>,
static_region: RegionVid,
static_region: Option<RegionVid>,
) -> Edges<'a, 'tcx, D> {
//if this is the `'static` region and the graph's direction is normal,
//then setup the Edges iterator to return all regions #53178
if region_sup == static_region && D::is_normal() {
if Some(region_sup) == static_region && D::is_normal() {
Edges {
graph: self,
constraints,
Expand All @@ -135,7 +137,7 @@ pub(crate) struct Edges<'a, 'tcx, D: ConstraintGraphDirection> {
constraints: &'a OutlivesConstraintSet<'tcx>,
pointer: Option<OutlivesConstraintIndex>,
next_static_idx: Option<usize>,
static_region: RegionVid,
static_region: Option<RegionVid>,
}

impl<'a, 'tcx, D: ConstraintGraphDirection> Iterator for Edges<'a, 'tcx, D> {
Expand All @@ -153,8 +155,12 @@ impl<'a, 'tcx, D: ConstraintGraphDirection> Iterator for Edges<'a, 'tcx, D> {
Some(next_static_idx + 1)
};

let Some(static_region) = self.static_region else {
return None;
};

Some(OutlivesConstraint {
sup: self.static_region,
sup: static_region,
sub: next_static_idx.into(),
locations: Locations::All(DUMMY_SP),
span: DUMMY_SP,
Expand Down Expand Up @@ -194,7 +200,11 @@ impl<'a, 'tcx, D: ConstraintGraphDirection> RegionGraph<'a, 'tcx, D> {
/// there exists a constraint `R: R1`.
pub(crate) fn outgoing_regions(&self, region_sup: RegionVid) -> Successors<'a, 'tcx, D> {
Successors {
edges: self.constraint_graph.outgoing_edges(region_sup, self.set, self.static_region),
edges: self.constraint_graph.outgoing_edges(
region_sup,
self.set,
Some(self.static_region),
),
}
}
}
Expand Down
125 changes: 29 additions & 96 deletions compiler/rustc_borrowck/src/constraints/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use std::fmt;
use std::ops::Index;

use rustc_data_structures::graph::scc;
use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::ty::{RegionVid, TyCtxt, VarianceDiagInfo};
use rustc_span::Span;
use tracing::{debug, instrument};
use tracing::debug;

use crate::region_infer::{ConstraintSccs, RegionDefinition, RegionTracker};
use crate::type_check::Locations;
use crate::universal_regions::UniversalRegions;

pub(crate) mod graph;

Expand Down Expand Up @@ -57,105 +56,39 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
/// Computes cycles (SCCs) in the graph of regions. In particular,
/// find all regions R1, R2 such that R1: R2 and R2: R1 and group
/// them into an SCC, and find the relationships between SCCs.
pub(crate) fn compute_sccs(
pub(crate) fn compute_sccs<
A: scc::Annotation,
AA: scc::Annotations<RegionVid, ConstraintSccIndex, A>,
>(
&self,
static_region: RegionVid,
definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
) -> ConstraintSccs {
let constraint_graph = self.graph(definitions.len());
num_region_vars: usize,
annotations: &mut AA,
) -> scc::Sccs<RegionVid, ConstraintSccIndex> {
let constraint_graph = self.graph(num_region_vars);
let region_graph = &constraint_graph.region_graph(self, static_region);
ConstraintSccs::new_with_annotation(&region_graph, |r| {
RegionTracker::new(r, &definitions[r])
})
scc::Sccs::new_with_annotation(&region_graph, annotations)
}

/// This method handles Universe errors by rewriting the constraint
/// graph. For each strongly connected component in the constraint
/// graph such that there is a series of constraints
/// A: B: C: ... : X where
/// A's universe is smaller than X's and A is a placeholder,
/// add a constraint that A: 'static. This is a safe upper bound
/// in the face of borrow checker/trait solver limitations that will
/// eventually go away.
///
/// For a more precise definition, see the documentation for
/// [`RegionTracker::has_incompatible_universes()`].
///
/// This edge case used to be handled during constraint propagation
/// by iterating over the strongly connected components in the constraint
/// graph while maintaining a set of bookkeeping mappings similar
/// to what is stored in `RegionTracker` and manually adding 'sttaic as
/// needed.
///
/// It was rewritten as part of the Polonius project with the goal of moving
/// higher-kindedness concerns out of the path of the borrow checker,
/// for two reasons:
///
/// 1. Implementing Polonius is difficult enough without also
/// handling them.
/// 2. The long-term goal is to handle higher-kinded concerns
/// in the trait solver, where they belong. This avoids
/// logic duplication and allows future trait solvers
/// to compute better bounds than for example our
/// "must outlive 'static" here.
///
/// This code is a stop-gap measure in preparation for the future trait solver.
///
/// Every constraint added by this method is an
/// internal `IllegalUniverse` constraint.
#[instrument(skip(self, universal_regions, definitions))]
pub(crate) fn add_outlives_static(
/// There is a placeholder violation; add a requirement
/// that some SCC outlive static and explain which region
/// reaching which other region caused that.
pub(crate) fn add_placeholder_violation_constraint(
&mut self,
universal_regions: &UniversalRegions<'tcx>,
definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
) -> ConstraintSccs {
let fr_static = universal_regions.fr_static;
let sccs = self.compute_sccs(fr_static, definitions);

// Changed to `true` if we added any constraints to `self` and need to
// recompute SCCs.
let mut added_constraints = false;

for scc in sccs.all_sccs() {
// No point in adding 'static: 'static!
// This micro-optimisation makes somewhat sense
// because static outlives *everything*.
if scc == sccs.scc(fr_static) {
continue;
}

let annotation = sccs.annotation(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.
added_constraints = true;
let scc_representative_outlives_static = OutlivesConstraint {
sup: annotation.representative,
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,
};
self.push(scc_representative_outlives_static);
}
}

if added_constraints {
// We changed the constraint set and so must recompute SCCs.
self.compute_sccs(fr_static, definitions)
} else {
// If we didn't add any back-edges; no more work needs doing
sccs
}
outlives_static: RegionVid,
blame_from: RegionVid,
blame_to: RegionVid,
fr_static: RegionVid,
) {
self.push(OutlivesConstraint {
sup: outlives_static,
sub: fr_static,
category: ConstraintCategory::IllegalPlaceholder(blame_from, blame_to),
locations: Locations::All(rustc_span::DUMMY_SP),
span: rustc_span::DUMMY_SP,
variance_info: VarianceDiagInfo::None,
from_closure: false,
});
}
}

Expand Down
Loading

0 comments on commit a87ebda

Please sign in to comment.