Skip to content

Commit

Permalink
Use annotions on the outlives-static constraints to
Browse files Browse the repository at this point in the history
help the search for who is to blame.
  • Loading branch information
amandasystems committed Sep 11, 2024
1 parent 222618c commit 4214d06
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 110 deletions.
9 changes: 5 additions & 4 deletions compiler/rustc_borrowck/src/constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,12 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {

let annotation = sccs.annotation(scc);

// If this SCC participates in a universe violation,
// 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
// the largest region reached, or if this placeholder
// reaches another placeholder, add a requirement that it must
// outlive `'static`.
if annotation.has_incompatible_universes() {
if let Some(offending_region) = annotation.placeholder_violation(&sccs) {
// 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
Expand All @@ -138,7 +139,7 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
let scc_representative_outlives_static = OutlivesConstraint {
sup: annotation.representative,
sub: fr_static,
category: ConstraintCategory::IllegalUniverse,
category: ConstraintCategory::IllegalPlaceholder(offending_region),
locations: Locations::All(rustc_span::DUMMY_SP),
span: rustc_span::DUMMY_SP,
variance_info: VarianceDiagInfo::None,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
let (blame_constraint, extra_info) = 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;

Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
| ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal
| ConstraintCategory::IllegalUniverse => "",
| ConstraintCategory::IllegalPlaceholder(_) => "",
}
}
}
Expand Down Expand Up @@ -430,9 +430,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);

let (blame_constraint, extra_info) =
self.regioncx.best_blame_constraint(fr, fr_origin, |r| {
self.regioncx.provides_universal_region(r, fr, outlived_fr)
});
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);
Expand Down
212 changes: 113 additions & 99 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,25 @@ enum RepresentativeOrigin {
Placeholder,
FreeRegion,
}

/// A reachable placeholder. Note the lexicographic ordering ensures
/// that they are ordered by:
/// A placeholder is larger than no placeholder, then
/// by universe, then
/// by region ID.
#[derive(Copy, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
enum ReachablePlaceholder {
Nothing,
Placeholder { universe: UniverseIndex, rvid: RegionVid },
}
/// An annotation for region graph SCCs that tracks
/// the values of its elements.
#[derive(Copy, Debug, Clone)]
pub struct RegionTracker {
/// The largest universe of a placeholder reached from this SCC.
/// This includes placeholders within this SCC.
max_placeholder_universe_reached: UniverseIndex,
/// This includes placeholders within this SCC. Including
/// the unverse's associated placeholder region ID.
max_universe_placeholder_reached: ReachablePlaceholder,

/// The smallest universe index reachable form the nodes of this SCC.
min_reachable_universe: UniverseIndex,
Expand Down Expand Up @@ -120,13 +132,16 @@ impl RegionTracker {

let rvid_is_placeholder = representative_origin == Placeholder;

let placeholder_universe =
if rvid_is_placeholder { definition.universe } else { UniverseIndex::ROOT };
let max_universe_placeholder_reached = if rvid_is_placeholder {
ReachablePlaceholder::Placeholder { universe: definition.universe, rvid }
} else {
ReachablePlaceholder::Nothing
};

let representative_if_placeholder = if rvid_is_placeholder { Some(rvid) } else { None };

Self {
max_placeholder_universe_reached: placeholder_universe,
max_universe_placeholder_reached,
min_reachable_universe: definition.universe,
representative: rvid,
representative_origin,
Expand Down Expand Up @@ -191,21 +206,67 @@ impl RegionTracker {
fn merge_min_max_seen(&mut self, other: &Self) {
self.merge_reachable_placeholders(other);

self.max_placeholder_universe_reached = std::cmp::max(
self.max_placeholder_universe_reached,
other.max_placeholder_universe_reached,
self.max_universe_placeholder_reached = std::cmp::max(
self.max_universe_placeholder_reached,
other.max_universe_placeholder_reached,
);

self.min_reachable_universe =
std::cmp::min(self.min_reachable_universe, other.min_reachable_universe);
}

/// Returns `true` if the annotated SCC reaches a placeholder
/// Returns an offending region if the annotated SCC reaches a placeholder
/// with a universe larger than the smallest reachable one,
/// or if a placeholder reaches another placeholder, `false` otherwise.
pub(crate) fn has_incompatible_universes(&self) -> bool {
self.min_universe().cannot_name(self.max_placeholder_universe_reached)
|| self.placeholder_reaches_placeholder()
/// or if a placeholder reaches another placeholder, `None` otherwise.
pub(crate) fn placeholder_violation(
&self,
sccs: &Sccs<RegionVid, ConstraintSccIndex, Self>,
) -> Option<RegionVid> {
// Note: we arbitrarily prefer universe violations
// to placeholder-reaches-placeholder violations.
// violations.

// Case 1: a universe violation
if let ReachablePlaceholder::Placeholder {
universe: max_reached_universe,
rvid: belonging_to_rvid,
} = self.max_universe_placeholder_reached
{
if self.min_universe().cannot_name(max_reached_universe) {
return Some(belonging_to_rvid);
}
}

// Case 2: a placeholder (in our SCC) reaches another placeholder
if self.placeholder_reaches_placeholder() {
// We know that this SCC contains at least one placeholder
// and that at least two placeholders are reachable from
// this SCC.
//
// We try to pick one that isn't in our SCC, if possible.
// We *always* pick one that is not equal to the representative.

// Unwrap safety: we know both these values are Some, since
// there are two reachable placeholders at least.
let min_reachable = self.min_reachable_placeholder.unwrap();

if sccs.scc(min_reachable) != sccs.scc(self.representative) {
return Some(min_reachable);
}

// Either the largest reachable placeholder is outside our SCC,
// or we *must* blame a placeholder in our SCC since the violation
// happens inside of it. It's slightly easier to always arbitrarily
// pick the largest one, so we do. This also nicely guarantees that
// we don't pick the representative, since the representative is the
// smallest placeholder by index in the SCC if it is a placeholder
// so in order for it to also be the largest reachable min would
// have to be equal to max, but then we would only have reached one
// placeholder.
return Some(self.max_reachable_placeholder.unwrap());
}

None
}
}

Expand Down Expand Up @@ -859,7 +920,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {

// Otherwise, there can be no placeholder in `b` with a too high
// universe index to name from `a`.
a_universe.can_name(b_annotation.max_placeholder_universe_reached)
if let ReachablePlaceholder::Placeholder { universe, .. } =
b_annotation.max_universe_placeholder_reached
{
a_universe.can_name(universe)
} else {
true
}
}

/// Once regions have been propagated, this method is used to see
Expand Down Expand Up @@ -1697,65 +1764,15 @@ 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(
&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;
let BlameConstraint { category, cause, .. } =
self.best_blame_constraint(fr1, fr1_origin, fr2).0;
(category, cause)
}

Expand Down Expand Up @@ -1837,10 +1854,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {

// This loop can be hot.
for constraint in outgoing_edges_from_graph {
if matches!(constraint.category, ConstraintCategory::IllegalUniverse) {
debug!("Ignoring illegal universe constraint: {constraint:?}");
continue;
}
handle_constraint(constraint);
}

Expand Down Expand Up @@ -1875,30 +1888,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
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()
}
Expand Down Expand Up @@ -1931,21 +1920,46 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}

/// Tries to find the best constraint to blame for the fact that
/// `R: from_region`, where `R` is some region that meets
/// `target_test`. This works by following the constraint graph,
/// `to_region: from_region`.
/// This works by following the constraint graph,
/// 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<ExtraConstraintInfo>) {
let result = self.best_blame_constraint_(from_region, from_region_origin, to_region);

// We are trying to blame an outlives-static constraint added
// by an issue with placeholder regions. We figure out why the placeholder
// region issue happened instead.
if let ConstraintCategory::IllegalPlaceholder(offending_r) = result.0.category {
debug!("best_blame_constraint: placeholder issue caused by {offending_r:?}");

if to_region == offending_r {
// We do not want an infinite loop.
return result;
}
return self.best_blame_constraint(from_region, from_region_origin, offending_r);
}

result
}

#[instrument(level = "debug", skip(self))]
pub(crate) fn best_blame_constraint_(
&self,
from_region: RegionVid,
from_region_origin: NllRegionVariableOrigin,
to_region: RegionVid,
) -> (BlameConstraint<'tcx>, Vec<ExtraConstraintInfo>) {
// Find all paths
let (path, target_region) =
self.find_constraint_paths_between_regions(from_region, target_test).unwrap();
self.find_constraint_paths_between_regions(from_region, |r| r == to_region).unwrap();
debug!(
"path={:#?}",
path.iter()
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisit
use rustc_span::symbol::Symbol;
use rustc_span::Span;
use rustc_target::abi::{FieldIdx, VariantIdx};
use rustc_type_ir::RegionVid;
use smallvec::SmallVec;

use super::{ConstValue, SourceInfo};
Expand Down Expand Up @@ -271,8 +272,9 @@ 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.
IllegalPlaceholder(RegionVid),
}

#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
Expand Down

0 comments on commit 4214d06

Please sign in to comment.