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.

Sign up for GitHub

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

Jump to bottom

Obligation forest tweaks #97674

Merged
merged 3 commits into from Jun 20, 2022
Merged

Obligation forest tweaks #97674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cdb446f
Streamline `active_cache` to `done_cache` transfer.
nnethercote Jun 3, 2022
281229a
Handle stalling within `ObligationForest`.
nnethercote Jun 1, 2022
32741d5
Split `process_obligation` in two.
nnethercote Jun 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
155 changes: 72 additions & 83 deletions compiler/rustc_data_structures/src/obligation_forest/mod.rs
View file
Open in desktop
Expand Up @@ -42,7 +42,7 @@
//! now considered to be in error.
//!
//! When the call to `process_obligations` completes, you get back an `Outcome`,
//! which includes three bits of information:
//! which includes two bits of information:
//!
//! - `completed`: a list of obligations where processing was fully
//! completed without error (meaning that all transitive subobligations
Expand All @@ -53,13 +53,10 @@
//! all the obligations in `C` have been found completed.
//! - `errors`: a list of errors that occurred and associated backtraces
//! at the time of error, which can be used to give context to the user.
//! - `stalled`: if true, then none of the existing obligations were
//! *shallowly successful* (that is, no callback returned `Changed(_)`).
//! This implies that all obligations were either errors or returned an
//! ambiguous result, which means that any further calls to
//! `process_obligations` would simply yield back further ambiguous
//! results. This is used by the `FulfillmentContext` to decide when it
//! has reached a steady state.
//!
//! Upon completion, none of the existing obligations were *shallowly
//! successful* (that is, no callback returned `Changed(_)`). This implies that
//! all obligations were either errors or returned an ambiguous result.
//!
//! ### Implementation details
//!
Expand Down Expand Up @@ -99,6 +96,8 @@ pub trait ObligationProcessor {
type Obligation: ForestObligation;
type Error: Debug;

fn needs_process_obligation(&self, obligation: &Self::Obligation) -> bool;

fn process_obligation(
&mut self,
obligation: &mut Self::Obligation,
Expand Down Expand Up @@ -146,7 +145,7 @@ pub struct ObligationForest<O: ForestObligation> {

/// A cache of the nodes in `nodes`, indexed by predicate. Unfortunately,
/// its contents are not guaranteed to match those of `nodes`. See the
/// comments in [`Self::process_obligation` for details.
/// comments in `Self::process_obligation` for details.
active_cache: FxHashMap<O::CacheKey, usize>,

/// A vector reused in [Self::compress()] and [Self::find_cycles_from_node()],
Expand Down Expand Up @@ -260,8 +259,6 @@ pub trait OutcomeTrait {
type Obligation;

fn new() -> Self;
fn mark_not_stalled(&mut self);
fn is_stalled(&self) -> bool;
fn record_completed(&mut self, outcome: &Self::Obligation);
fn record_error(&mut self, error: Self::Error);
}
Expand All @@ -270,30 +267,14 @@ pub trait OutcomeTrait {
pub struct Outcome<O, E> {
/// Backtrace of obligations that were found to be in error.
pub errors: Vec<Error<O, E>>,

/// If true, then we saw no successful obligations, which means
/// there is no point in further iteration. This is based on the
/// assumption that when trait matching returns `Error` or
/// `Unchanged`, those results do not affect environmental
/// inference state. (Note that if we invoke `process_obligations`
/// with no pending obligations, stalled will be true.)
pub stalled: bool,
}

impl<O, E> OutcomeTrait for Outcome<O, E> {
type Error = Error<O, E>;
type Obligation = O;

fn new() -> Self {
Self { stalled: true, errors: vec![] }
}

fn mark_not_stalled(&mut self) {
self.stalled = false;
}

fn is_stalled(&self) -> bool {
self.stalled
Self { errors: vec![] }
}

fn record_completed(&mut self, _outcome: &Self::Obligation) {
Expand Down Expand Up @@ -415,10 +396,7 @@ impl<O: ForestObligation> ObligationForest<O> {
.insert(node.obligation.as_cache_key());
}

/// Performs a pass through the obligation list. This must
/// be called in a loop until `outcome.stalled` is false.
///
/// This _cannot_ be unrolled (presently, at least).
/// Performs a fixpoint computation over the obligation list.
#[inline(never)]
pub fn process_obligations<P, OUT>(&mut self, processor: &mut P) -> OUT
where
Expand All @@ -427,55 +405,69 @@ impl<O: ForestObligation> ObligationForest<O> {
{
let mut outcome = OUT::new();

// Note that the loop body can append new nodes, and those new nodes
// will then be processed by subsequent iterations of the loop.
//
// We can't use an iterator for the loop because `self.nodes` is
// appended to and the borrow checker would complain. We also can't use
// `for index in 0..self.nodes.len() { ... }` because the range would
// be computed with the initial length, and we would miss the appended
// nodes. Therefore we use a `while` loop.
let mut index = 0;
while let Some(node) = self.nodes.get_mut(index) {
// `processor.process_obligation` can modify the predicate within
// `node.obligation`, and that predicate is the key used for
// `self.active_cache`. This means that `self.active_cache` can get
// out of sync with `nodes`. It's not very common, but it does
// happen, and code in `compress` has to allow for it.
if node.state.get() != NodeState::Pending {
index += 1;
continue;
}

match processor.process_obligation(&mut node.obligation) {
ProcessResult::Unchanged => {
// No change in state.
// Fixpoint computation: we repeat until the inner loop stalls.
loop {
let mut has_changed = false;

// Note that the loop body can append new nodes, and those new nodes
// will then be processed by subsequent iterations of the loop.
//
// We can't use an iterator for the loop because `self.nodes` is
// appended to and the borrow checker would complain. We also can't use
// `for index in 0..self.nodes.len() { ... }` because the range would
// be computed with the initial length, and we would miss the appended
// nodes. Therefore we use a `while` loop.
let mut index = 0;
while let Some(node) = self.nodes.get_mut(index) {
if node.state.get() != NodeState::Pending
|| !processor.needs_process_obligation(&node.obligation)
{
index += 1;
continue;
}
ProcessResult::Changed(children) => {
// We are not (yet) stalled.
outcome.mark_not_stalled();
node.state.set(NodeState::Success);

for child in children {
let st = self.register_obligation_at(child, Some(index));
if let Err(()) = st {
// Error already reported - propagate it
// to our node.
self.error_at(index);

// `processor.process_obligation` can modify the predicate within
// `node.obligation`, and that predicate is the key used for
// `self.active_cache`. This means that `self.active_cache` can get
// out of sync with `nodes`. It's not very common, but it does
// happen, and code in `compress` has to allow for it.

match processor.process_obligation(&mut node.obligation) {
ProcessResult::Unchanged => {
// No change in state.
}
ProcessResult::Changed(children) => {
// We are not (yet) stalled.
has_changed = true;
node.state.set(NodeState::Success);

for child in children {
let st = self.register_obligation_at(child, Some(index));
if let Err(()) = st {
// Error already reported - propagate it
// to our node.
self.error_at(index);
}
}
}
ProcessResult::Error(err) => {
has_changed = true;
outcome.record_error(Error { error: err, backtrace: self.error_at(index) });
}
}
ProcessResult::Error(err) => {
outcome.mark_not_stalled();
outcome.record_error(Error { error: err, backtrace: self.error_at(index) });
}
index += 1;
}

// If unchanged, then we saw no successful obligations, which means
// there is no point in further iteration. This is based on the
// assumption that when trait matching returns `Error` or
// `Unchanged`, those results do not affect environmental inference
// state. (Note that this will occur if we invoke
// `process_obligations` with no pending obligations.)
if !has_changed {
break;
}
index += 1;
}

// There's no need to perform marking, cycle processing and compression when nothing
// changed.
if !outcome.is_stalled() {
self.mark_successes();
self.process_cycles(processor);
self.compress(|obl| outcome.record_completed(obl));
Expand Down Expand Up @@ -634,17 +626,14 @@ impl<O: ForestObligation> ObligationForest<O> {
}
}
NodeState::Done => {
// This lookup can fail because the contents of
// The removal lookup might fail because the contents of
// `self.active_cache` are not guaranteed to match those of
// `self.nodes`. See the comment in `process_obligation`
// for more details.
if let Some((predicate, _)) =
self.active_cache.remove_entry(&node.obligation.as_cache_key())
{
self.done_cache.insert(predicate);
} else {
self.done_cache.insert(node.obligation.as_cache_key().clone());
}
let cache_key = node.obligation.as_cache_key();
self.active_cache.remove(&cache_key);
self.done_cache.insert(cache_key);

// Extract the success stories.
outcome_cb(&node.obligation);
node_rewrites[index] = orig_nodes_len;
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_data_structures/src/obligation_forest/tests.rs
View file
Open in desktop
Expand Up @@ -20,7 +20,6 @@ struct ClosureObligationProcessor<OF, BF, O, E> {
struct TestOutcome<O, E> {
pub completed: Vec<O>,
pub errors: Vec<Error<O, E>>,
pub stalled: bool,
}

impl<O, E> OutcomeTrait for TestOutcome<O, E>
Expand All @@ -31,15 +30,7 @@ where
type Obligation = O;

fn new() -> Self {
Self { errors: vec![], stalled: false, completed: vec![] }
}

fn mark_not_stalled(&mut self) {
self.stalled = false;
}

fn is_stalled(&self) -> bool {
self.stalled
Self { errors: vec![], completed: vec![] }
}

fn record_completed(&mut self, outcome: &Self::Obligation) {
Expand Down Expand Up @@ -74,6 +65,10 @@ where
type Obligation = O;
type Error = E;

fn needs_process_obligation(&self, _obligation: &Self::Obligation) -> bool {
true
}

fn process_obligation(
&mut self,
obligation: &mut Self::Obligation,
Expand Down