Skip to content

Commit 985fcce

Browse files
committed
Use annotions on the outlives-static constraints to
help the search for who is to blame.
1 parent 6a4deba commit 985fcce

File tree

5 files changed

+125
-110
lines changed

5 files changed

+125
-110
lines changed

compiler/rustc_borrowck/src/constraints/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,12 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
126126

127127
let annotation = sccs.annotation(scc);
128128

129-
// If this SCC participates in a universe violation,
129+
// If this SCC participates in a universe violation
130130
// e.g. if it reaches a region with a universe smaller than
131-
// the largest region reached, add a requirement that it must
131+
// the largest region reached, or if this placeholder
132+
// reaches another placeholder, add a requirement that it must
132133
// outlive `'static`.
133-
if annotation.has_incompatible_universes() {
134+
if let Some(offending_region) = annotation.placeholder_violation(&sccs) {
134135
// Optimisation opportunity: this will add more constraints than
135136
// needed for correctness, since an SCC upstream of another with
136137
// a universe violation will "infect" its downstream SCCs to also
@@ -139,7 +140,7 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
139140
let scc_representative_outlives_static = OutlivesConstraint {
140141
sup: annotation.representative,
141142
sub: fr_static,
142-
category: ConstraintCategory::IllegalUniverse,
143+
category: ConstraintCategory::IllegalPlaceholder(offending_region),
143144
locations: Locations::All(rustc_span::DUMMY_SP),
144145
span: rustc_span::DUMMY_SP,
145146
variance_info: VarianceDiagInfo::None,

compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
488488
let (blame_constraint, extra_info) = self.regioncx.best_blame_constraint(
489489
borrow_region,
490490
NllRegionVariableOrigin::FreeRegion,
491-
|r| self.regioncx.provides_universal_region(r, borrow_region, outlived_region),
491+
outlived_region,
492492
);
493493
let BlameConstraint { category, from_closure, cause, .. } = blame_constraint;
494494

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ impl<'tcx> ConstraintDescription for ConstraintCategory<'tcx> {
6161
| ConstraintCategory::Boring
6262
| ConstraintCategory::BoringNoLocation
6363
| ConstraintCategory::Internal
64-
| ConstraintCategory::IllegalUniverse => "",
64+
| ConstraintCategory::IllegalPlaceholder(_) => "",
6565
}
6666
}
6767
}
@@ -432,9 +432,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
432432
debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);
433433

434434
let (blame_constraint, extra_info) =
435-
self.regioncx.best_blame_constraint(fr, fr_origin, |r| {
436-
self.regioncx.provides_universal_region(r, fr, outlived_fr)
437-
});
435+
self.regioncx.best_blame_constraint(fr, fr_origin, outlived_fr);
438436
let BlameConstraint { category, cause, variance_info, .. } = blame_constraint;
439437

440438
debug!("report_region_error: category={:?} {:?} {:?}", category, cause, variance_info);

compiler/rustc_borrowck/src/region_infer/mod.rs

Lines changed: 113 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,25 @@ enum RepresentativeOrigin {
5555
Placeholder,
5656
FreeRegion,
5757
}
58+
59+
/// A reachable placeholder. Note the lexicographic ordering ensures
60+
/// that they are ordered by:
61+
/// A placeholder is larger than no placeholder, then
62+
/// by universe, then
63+
/// by region ID.
64+
#[derive(Copy, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
65+
enum ReachablePlaceholder {
66+
Nothing,
67+
Placeholder { universe: UniverseIndex, rvid: RegionVid },
68+
}
5869
/// An annotation for region graph SCCs that tracks
5970
/// the values of its elements.
6071
#[derive(Copy, Debug, Clone)]
6172
pub struct RegionTracker {
6273
/// The largest universe of a placeholder reached from this SCC.
63-
/// This includes placeholders within this SCC.
64-
max_placeholder_universe_reached: UniverseIndex,
74+
/// This includes placeholders within this SCC. Including
75+
/// the unverse's associated placeholder region ID.
76+
max_universe_placeholder_reached: ReachablePlaceholder,
6577

6678
/// The smallest universe index reachable form the nodes of this SCC.
6779
min_reachable_universe: UniverseIndex,
@@ -121,13 +133,16 @@ impl RegionTracker {
121133

122134
let rvid_is_placeholder = representative_origin == Placeholder;
123135

124-
let placeholder_universe =
125-
if rvid_is_placeholder { definition.universe } else { UniverseIndex::ROOT };
136+
let max_universe_placeholder_reached = if rvid_is_placeholder {
137+
ReachablePlaceholder::Placeholder { universe: definition.universe, rvid }
138+
} else {
139+
ReachablePlaceholder::Nothing
140+
};
126141

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

129144
Self {
130-
max_placeholder_universe_reached: placeholder_universe,
145+
max_universe_placeholder_reached,
131146
min_reachable_universe: definition.universe,
132147
representative: rvid,
133148
representative_origin,
@@ -192,21 +207,67 @@ impl RegionTracker {
192207
fn merge_min_max_seen(&mut self, other: &Self) {
193208
self.merge_reachable_placeholders(other);
194209

195-
self.max_placeholder_universe_reached = std::cmp::max(
196-
self.max_placeholder_universe_reached,
197-
other.max_placeholder_universe_reached,
210+
self.max_universe_placeholder_reached = std::cmp::max(
211+
self.max_universe_placeholder_reached,
212+
other.max_universe_placeholder_reached,
198213
);
199214

200215
self.min_reachable_universe =
201216
std::cmp::min(self.min_reachable_universe, other.min_reachable_universe);
202217
}
203218

204-
/// Returns `true` if the annotated SCC reaches a placeholder
219+
/// Returns an offending region if the annotated SCC reaches a placeholder
205220
/// with a universe larger than the smallest reachable one,
206-
/// or if a placeholder reaches another placeholder, `false` otherwise.
207-
pub(crate) fn has_incompatible_universes(&self) -> bool {
208-
self.min_universe().cannot_name(self.max_placeholder_universe_reached)
209-
|| self.placeholder_reaches_placeholder()
221+
/// or if a placeholder reaches another placeholder, `None` otherwise.
222+
pub(crate) fn placeholder_violation(
223+
&self,
224+
sccs: &Sccs<RegionVid, ConstraintSccIndex, Self>,
225+
) -> Option<RegionVid> {
226+
// Note: we arbitrarily prefer universe violations
227+
// to placeholder-reaches-placeholder violations.
228+
// violations.
229+
230+
// Case 1: a universe violation
231+
if let ReachablePlaceholder::Placeholder {
232+
universe: max_reached_universe,
233+
rvid: belonging_to_rvid,
234+
} = self.max_universe_placeholder_reached
235+
{
236+
if self.min_universe().cannot_name(max_reached_universe) {
237+
return Some(belonging_to_rvid);
238+
}
239+
}
240+
241+
// Case 2: a placeholder (in our SCC) reaches another placeholder
242+
if self.placeholder_reaches_placeholder() {
243+
// We know that this SCC contains at least one placeholder
244+
// and that at least two placeholders are reachable from
245+
// this SCC.
246+
//
247+
// We try to pick one that isn't in our SCC, if possible.
248+
// We *always* pick one that is not equal to the representative.
249+
250+
// Unwrap safety: we know both these values are Some, since
251+
// there are two reachable placeholders at least.
252+
let min_reachable = self.min_reachable_placeholder.unwrap();
253+
254+
if sccs.scc(min_reachable) != sccs.scc(self.representative) {
255+
return Some(min_reachable);
256+
}
257+
258+
// Either the largest reachable placeholder is outside our SCC,
259+
// or we *must* blame a placeholder in our SCC since the violation
260+
// happens inside of it. It's slightly easier to always arbitrarily
261+
// pick the largest one, so we do. This also nicely guarantees that
262+
// we don't pick the representative, since the representative is the
263+
// smallest placeholder by index in the SCC if it is a placeholder
264+
// so in order for it to also be the largest reachable min would
265+
// have to be equal to max, but then we would only have reached one
266+
// placeholder.
267+
return Some(self.max_reachable_placeholder.unwrap());
268+
}
269+
270+
None
210271
}
211272
}
212273

@@ -860,7 +921,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
860921

861922
// Otherwise, there can be no placeholder in `b` with a too high
862923
// universe index to name from `a`.
863-
a_universe.can_name(b_annotation.max_placeholder_universe_reached)
924+
if let ReachablePlaceholder::Placeholder { universe, .. } =
925+
b_annotation.max_universe_placeholder_reached
926+
{
927+
a_universe.can_name(universe)
928+
} else {
929+
true
930+
}
864931
}
865932

866933
/// Once regions have been propagated, this method is used to see
@@ -1698,65 +1765,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
16981765
}
16991766
}
17001767

1701-
/// We have a constraint `fr1: fr2` that is not satisfied, where
1702-
/// `fr2` represents some universal region. Here, `r` is some
1703-
/// region where we know that `fr1: r` and this function has the
1704-
/// job of determining whether `r` is "to blame" for the fact that
1705-
/// `fr1: fr2` is required.
1706-
///
1707-
/// This is true under two conditions:
1708-
///
1709-
/// - `r == fr2`
1710-
/// - `fr2` is `'static` and `r` is some placeholder in a universe
1711-
/// that cannot be named by `fr1`; in that case, we will require
1712-
/// that `fr1: 'static` because it is the only way to `fr1: r` to
1713-
/// be satisfied. (See `add_incompatible_universe`.)
1714-
pub(crate) fn provides_universal_region(
1715-
&self,
1716-
r: RegionVid,
1717-
fr1: RegionVid,
1718-
fr2: RegionVid,
1719-
) -> bool {
1720-
debug!("provides_universal_region(r={:?}, fr1={:?}, fr2={:?})", r, fr1, fr2);
1721-
let result = {
1722-
r == fr2 || {
1723-
fr2 == self.universal_regions.fr_static && self.cannot_name_placeholder(fr1, r)
1724-
}
1725-
};
1726-
debug!("provides_universal_region: result = {:?}", result);
1727-
result
1728-
}
1729-
1730-
/// If `r2` represents a placeholder region, then this returns
1731-
/// `true` if `r1` cannot name that placeholder in its
1732-
/// value; otherwise, returns `false`.
1733-
pub(crate) fn cannot_name_placeholder(&self, r1: RegionVid, r2: RegionVid) -> bool {
1734-
match self.definitions[r2].origin {
1735-
NllRegionVariableOrigin::Placeholder(placeholder) => {
1736-
let r1_universe = self.definitions[r1].universe;
1737-
debug!(
1738-
"cannot_name_value_of: universe1={r1_universe:?} placeholder={:?}",
1739-
placeholder
1740-
);
1741-
r1_universe.cannot_name(placeholder.universe)
1742-
}
1743-
1744-
NllRegionVariableOrigin::FreeRegion | NllRegionVariableOrigin::Existential { .. } => {
1745-
false
1746-
}
1747-
}
1748-
}
1749-
17501768
/// Finds a good `ObligationCause` to blame for the fact that `fr1` outlives `fr2`.
17511769
pub(crate) fn find_outlives_blame_span(
17521770
&self,
17531771
fr1: RegionVid,
17541772
fr1_origin: NllRegionVariableOrigin,
17551773
fr2: RegionVid,
17561774
) -> (ConstraintCategory<'tcx>, ObligationCause<'tcx>) {
1757-
let BlameConstraint { category, cause, .. } = self
1758-
.best_blame_constraint(fr1, fr1_origin, |r| self.provides_universal_region(r, fr1, fr2))
1759-
.0;
1775+
let BlameConstraint { category, cause, .. } =
1776+
self.best_blame_constraint(fr1, fr1_origin, fr2).0;
17601777
(category, cause)
17611778
}
17621779

@@ -1838,10 +1855,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
18381855

18391856
// This loop can be hot.
18401857
for constraint in outgoing_edges_from_graph {
1841-
if matches!(constraint.category, ConstraintCategory::IllegalUniverse) {
1842-
debug!("Ignoring illegal universe constraint: {constraint:?}");
1843-
continue;
1844-
}
18451858
handle_constraint(constraint);
18461859
}
18471860

@@ -1876,30 +1889,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
18761889
trace!(?r, liveness_constraints=?self.liveness_constraints.pretty_print_live_points(r));
18771890
self.liveness_constraints.is_live_at(r, location)
18781891
})
1879-
.or_else(|| {
1880-
// If we fail to find that, we may find some `r` such that
1881-
// `fr1: r` and `r` is a placeholder from some universe
1882-
// `fr1` cannot name. This would force `fr1` to be
1883-
// `'static`.
1884-
self.find_constraint_paths_between_regions(fr1, |r| {
1885-
self.cannot_name_placeholder(fr1, r)
1886-
})
1887-
})
1888-
.or_else(|| {
1889-
// If we fail to find THAT, it may be that `fr1` is a
1890-
// placeholder that cannot "fit" into its SCC. In that
1891-
// case, there should be some `r` where `fr1: r` and `fr1` is a
1892-
// placeholder that `r` cannot name. We can blame that
1893-
// edge.
1894-
//
1895-
// Remember that if `R1: R2`, then the universe of R1
1896-
// must be able to name the universe of R2, because R2 will
1897-
// be at least `'empty(Universe(R2))`, and `R1` must be at
1898-
// larger than that.
1899-
self.find_constraint_paths_between_regions(fr1, |r| {
1900-
self.cannot_name_placeholder(r, fr1)
1901-
})
1902-
})
19031892
.map(|(_path, r)| r)
19041893
.unwrap()
19051894
}
@@ -1932,21 +1921,46 @@ impl<'tcx> RegionInferenceContext<'tcx> {
19321921
}
19331922

19341923
/// Tries to find the best constraint to blame for the fact that
1935-
/// `R: from_region`, where `R` is some region that meets
1936-
/// `target_test`. This works by following the constraint graph,
1924+
/// `to_region: from_region`.
1925+
/// This works by following the constraint graph,
19371926
/// creating a constraint path that forces `R` to outlive
19381927
/// `from_region`, and then finding the best choices within that
19391928
/// path to blame.
1940-
#[instrument(level = "debug", skip(self, target_test))]
1929+
#[instrument(level = "debug", skip(self))]
19411930
pub(crate) fn best_blame_constraint(
19421931
&self,
19431932
from_region: RegionVid,
19441933
from_region_origin: NllRegionVariableOrigin,
1945-
target_test: impl Fn(RegionVid) -> bool,
1934+
to_region: RegionVid,
1935+
) -> (BlameConstraint<'tcx>, Vec<ExtraConstraintInfo>) {
1936+
let result = self.best_blame_constraint_(from_region, from_region_origin, to_region);
1937+
1938+
// We are trying to blame an outlives-static constraint added
1939+
// by an issue with placeholder regions. We figure out why the placeholder
1940+
// region issue happened instead.
1941+
if let ConstraintCategory::IllegalPlaceholder(offending_r) = result.0.category {
1942+
debug!("best_blame_constraint: placeholder issue caused by {offending_r:?}");
1943+
1944+
if to_region == offending_r {
1945+
// We do not want an infinite loop.
1946+
return result;
1947+
}
1948+
return self.best_blame_constraint(from_region, from_region_origin, offending_r);
1949+
}
1950+
1951+
result
1952+
}
1953+
1954+
#[instrument(level = "debug", skip(self))]
1955+
pub(crate) fn best_blame_constraint_(
1956+
&self,
1957+
from_region: RegionVid,
1958+
from_region_origin: NllRegionVariableOrigin,
1959+
to_region: RegionVid,
19461960
) -> (BlameConstraint<'tcx>, Vec<ExtraConstraintInfo>) {
19471961
// Find all paths
19481962
let (path, target_region) =
1949-
self.find_constraint_paths_between_regions(from_region, target_test).unwrap();
1963+
self.find_constraint_paths_between_regions(from_region, |r| r == to_region).unwrap();
19501964
debug!(
19511965
"path={:#?}",
19521966
path.iter()

compiler/rustc_middle/src/mir/query.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisit
1313
use rustc_span::Span;
1414
use rustc_span::symbol::Symbol;
1515
use rustc_target::abi::{FieldIdx, VariantIdx};
16+
use rustc_type_ir::RegionVid;
1617
use smallvec::SmallVec;
1718

1819
use super::{ConstValue, SourceInfo};
@@ -273,8 +274,9 @@ pub enum ConstraintCategory<'tcx> {
273274
/// A constraint that doesn't correspond to anything the user sees.
274275
Internal,
275276

276-
/// An internal constraint derived from an illegal universe relation.
277-
IllegalUniverse,
277+
/// An internal constraint derived from an illegal placeholder relation
278+
/// to this region.
279+
IllegalPlaceholder(RegionVid),
278280
}
279281

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

0 commit comments

Comments
 (0)