Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove placeholders completely #130227

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 11 additions & 99 deletions compiler/rustc_borrowck/src/constraints/mod.rs
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;

@@ -57,105 +56,18 @@ 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])
})
}

/// 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(
&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
}
scc::Sccs::new_with_annotation(&region_graph, annotations)
}
}

105 changes: 52 additions & 53 deletions compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs
Original file line number Diff line number Diff line change
@@ -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,12 +48,13 @@ impl<'tcx> UniverseInfo<'tcx> {
UniverseInfo::RelateTys { expected, found }
}

pub(crate) fn report_error(
/// 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,
cause: ObligationCause<'tcx>,
error_element: Option<ty::PlaceholderRegion>,
) {
match *self {
UniverseInfo::RelateTys { expected, found } => {
@@ -68,7 +68,7 @@ impl<'tcx> UniverseInfo<'tcx> {
mbcx.buffer_error(err);
}
UniverseInfo::TypeOp(ref type_op_info) => {
type_op_info.report_error(mbcx, placeholder, error_element, cause);
type_op_info.report_erroneous_element(mbcx, placeholder, cause, error_element);
}
UniverseInfo::Other => {
// FIXME: This error message isn't great, but it doesn't show
@@ -145,57 +145,57 @@ pub(crate) trait TypeOpInfo<'tcx> {
error_region: Option<ty::Region<'tcx>>,
) -> Option<Diag<'infcx>>;

/// 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 figure out what happened later.
#[instrument(level = "debug", skip(self, mbcx))]
fn report_error(
fn report_erroneous_element(
&self,
mbcx: &mut MirBorrowckCtxt<'_, '_, 'tcx>,
placeholder: ty::PlaceholderRegion,
error_element: RegionElement,
cause: ObligationCause<'tcx>,
error_element: Option<ty::PlaceholderRegion>,
) {
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);

if let Some(nice_error) = nice_error {
mbcx.buffer_error(nice_error);
} else {
mbcx.buffer_error(self.fallback_error(tcx, span));
}
mbcx.buffer_error(nice_error.unwrap_or_else(|| self.fallback_error(tcx, span)));
}
}

@@ -450,7 +450,8 @@ fn try_extract_error_from_region_constraints<'a, 'tcx>(
ty::ReVar(vid) => universe_of_region(vid),
_ => ty::UniverseIndex::ROOT,
};
let matches =
// Are the two regions the same?
let regions_the_same =
|a_region: Region<'tcx>, b_region: Region<'tcx>| match (a_region.kind(), b_region.kind()) {
(RePlaceholder(a_p), RePlaceholder(b_p)) => a_p.bound == b_p.bound,
_ => a_region == b_region,
@@ -459,7 +460,7 @@ fn try_extract_error_from_region_constraints<'a, 'tcx>(
|constraint: &Constraint<'tcx>, cause: &SubregionOrigin<'tcx>, exact| match *constraint {
Constraint::RegSubReg(sub, sup)
if ((exact && sup == placeholder_region)
|| (!exact && matches(sup, placeholder_region)))
|| (!exact && regions_the_same(sup, placeholder_region)))
&& sup != sub =>
{
Some((sub, cause.clone()))
@@ -468,23 +469,21 @@ fn try_extract_error_from_region_constraints<'a, 'tcx>(
if (exact
&& sup == placeholder_region
&& !universe_of_region(vid).can_name(placeholder_universe))
|| (!exact && matches(sup, placeholder_region)) =>
|| (!exact && regions_the_same(sup, placeholder_region)) =>
{
Some((ty::Region::new_var(infcx.tcx, vid), cause.clone()))
}
_ => None,
};
let mut info = region_constraints
.constraints
.iter()
.find_map(|(constraint, cause)| check(constraint, cause, true));
if info.is_none() {
info = region_constraints

let mut find_culprit = |exact_match: bool| {
region_constraints
.constraints
.iter()
.find_map(|(constraint, cause)| check(constraint, cause, false));
}
let (sub_region, cause) = info?;
.find_map(|(constraint, cause)| check(constraint, cause, exact_match))
};

let (sub_region, cause) = find_culprit(true).or_else(|| find_culprit(false))?;

debug!(?sub_region, "cause = {:#?}", cause);
let error = match (error_region, *sub_region) {
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
@@ -563,7 +563,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;

7 changes: 3 additions & 4 deletions compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs
Original file line number Diff line number Diff line change
@@ -174,10 +174,9 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> 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.
Loading
Loading