From 44a23888288d680ccfd24409629e184fe8ab95bb Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 31 Jan 2023 13:42:00 -0300 Subject: [PATCH 1/5] Make Ok value of repeat_while_none more general --- .../rustc_trait_selection/src/solve/mod.rs | 49 ++++++++++--------- .../src/solve/search_graph/overflow.rs | 9 ++-- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index e56588c58bd..e3f8f7cddab 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -485,35 +485,38 @@ fn evaluate_all( mut goals: Vec>>, ) -> Result { let mut new_goals = Vec::new(); - self.repeat_while_none(|this| { - let mut has_changed = Err(Certainty::Yes); - for goal in goals.drain(..) { - let (changed, certainty) = match this.evaluate_goal(goal) { - Ok(result) => result, - Err(NoSolution) => return Some(Err(NoSolution)), - }; + self.repeat_while_none( + |_| Certainty::Maybe(MaybeCause::Overflow), + |this| { + let mut has_changed = Err(Certainty::Yes); + for goal in goals.drain(..) { + let (changed, certainty) = match this.evaluate_goal(goal) { + Ok(result) => result, + Err(NoSolution) => return Some(Err(NoSolution)), + }; - if changed { - has_changed = Ok(()); - } + if changed { + has_changed = Ok(()); + } - match certainty { - Certainty::Yes => {} - Certainty::Maybe(_) => { - new_goals.push(goal); - has_changed = has_changed.map_err(|c| c.unify_and(certainty)); + match certainty { + Certainty::Yes => {} + Certainty::Maybe(_) => { + new_goals.push(goal); + has_changed = has_changed.map_err(|c| c.unify_and(certainty)); + } } } - } - match has_changed { - Ok(()) => { - mem::swap(&mut new_goals, &mut goals); - None + match has_changed { + Ok(()) => { + mem::swap(&mut new_goals, &mut goals); + None + } + Err(certainty) => Some(Ok(certainty)), } - Err(certainty) => Some(Ok(certainty)), - } - }) + }, + ) } // Recursively evaluates a list of goals to completion, making a query response. diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs index 1dd3894c91a..c472dfe5a00 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs @@ -63,10 +63,11 @@ pub fn deal_with_overflow( impl<'tcx> EvalCtxt<'_, 'tcx> { /// A `while`-loop which tracks overflow. - pub fn repeat_while_none( + pub fn repeat_while_none( &mut self, - mut loop_body: impl FnMut(&mut Self) -> Option>, - ) -> Result { + mut overflow_body: impl FnMut(&mut Self) -> T, + mut loop_body: impl FnMut(&mut Self) -> Option>, + ) -> Result { let start_depth = self.search_graph.overflow_data.additional_depth; let depth = self.search_graph.stack.len(); while !self.search_graph.overflow_data.has_overflow(depth) { @@ -79,6 +80,6 @@ pub fn repeat_while_none( } self.search_graph.overflow_data.additional_depth = start_depth; self.search_graph.overflow_data.deal_with_overflow(); - Ok(Certainty::Maybe(MaybeCause::Overflow)) + Ok(overflow_body(self)) } } From 873c83ba56650a32383bb8fb4820a0ce792bc121 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 13 Feb 2023 14:37:46 -0300 Subject: [PATCH 2/5] Extract try_move_finished_goal_to_global_cache from try_finalize_goal --- .../src/solve/search_graph/mod.rs | 57 +++++++++++-------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs index a2ca4bc189c..c25a9cddfe7 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs @@ -133,7 +133,6 @@ pub(super) fn try_finalize_goal( let cache = &mut self.provisional_cache; let provisional_entry_index = *cache.lookup_table.get(&goal).unwrap(); let provisional_entry = &mut cache.entries[provisional_entry_index]; - let depth = provisional_entry.depth; // We eagerly update the response in the cache here. If we have to reevaluate // this goal we use the new response when hitting a cycle, and we definitely // want to access the final response whenever we look at the cache. @@ -157,29 +156,41 @@ pub(super) fn try_finalize_goal( self.stack.push(StackElem { goal, has_been_used: false }); false } else { - // If not, we're done with this goal. - // - // Check whether that this goal doesn't depend on a goal deeper on the stack - // and if so, move it and all nested goals to the global cache. - // - // Note that if any nested goal were to depend on something deeper on the stack, - // this would have also updated the depth of the current goal. - if depth == self.stack.next_index() { - for (i, entry) in cache.entries.drain_enumerated(provisional_entry_index.index()..) - { - let actual_index = cache.lookup_table.remove(&entry.goal); - debug_assert_eq!(Some(i), actual_index); - debug_assert!(entry.depth == depth); - cache::try_move_finished_goal_to_global_cache( - tcx, - &mut self.overflow_data, - &self.stack, - entry.goal, - entry.response, - ); - } - } + self.try_move_finished_goal_to_global_cache(tcx, &goal); true } } + + pub(super) fn try_move_finished_goal_to_global_cache( + &mut self, + tcx: TyCtxt<'tcx>, + goal: &CanonicalGoal<'tcx>, + ) { + let cache = &mut self.provisional_cache; + let provisional_entry_index = *cache.lookup_table.get(goal).unwrap(); + let provisional_entry = &mut cache.entries[provisional_entry_index]; + let depth = provisional_entry.depth; + + // If not, we're done with this goal. + // + // Check whether that this goal doesn't depend on a goal deeper on the stack + // and if so, move it and all nested goals to the global cache. + // + // Note that if any nested goal were to depend on something deeper on the stack, + // this would have also updated the depth of the current goal. + if depth == self.stack.next_index() { + for (i, entry) in cache.entries.drain_enumerated(provisional_entry_index.index()..) { + let actual_index = cache.lookup_table.remove(&entry.goal); + debug_assert_eq!(Some(i), actual_index); + debug_assert!(entry.depth == depth); + cache::try_move_finished_goal_to_global_cache( + tcx, + &mut self.overflow_data, + &self.stack, + entry.goal, + entry.response, + ); + } + } + } } From 826bee7085620843d184dda382b1aa825fc4b770 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 6 Feb 2023 16:28:27 -0300 Subject: [PATCH 3/5] Implement repeat_while_none for both SearchGraph and EvalCtxt --- .../rustc_trait_selection/src/solve/mod.rs | 1 + .../src/solve/search_graph/mod.rs | 4 +- .../src/solve/search_graph/overflow.rs | 59 +++++++++++-------- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index e3f8f7cddab..358a2bcc7b9 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -31,6 +31,7 @@ }; use rustc_span::DUMMY_SP; +use crate::solve::search_graph::overflow::OverflowHandler; use crate::traits::ObligationCause; mod assembly; diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs index c25a9cddfe7..438bcd9a7d6 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs @@ -1,5 +1,5 @@ mod cache; -mod overflow; +pub(crate) mod overflow; use self::cache::ProvisionalEntry; use super::{CanonicalGoal, Certainty, MaybeCause, QueryResult}; @@ -18,7 +18,7 @@ struct StackElem<'tcx> { has_been_used: bool, } -pub(super) struct SearchGraph<'tcx> { +pub(crate) struct SearchGraph<'tcx> { /// The stack of goals currently being computed. /// /// An element is *deeper* in the stack if its index is *lower*. diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs index c472dfe5a00..0d6863b1e81 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs @@ -50,6 +50,42 @@ fn deal_with_overflow(&mut self) { } } +pub(crate) trait OverflowHandler<'tcx> { + fn search_graph(&mut self) -> &mut SearchGraph<'tcx>; + + fn repeat_while_none( + &mut self, + on_overflow: impl FnOnce(&mut Self) -> T, + mut loop_body: impl FnMut(&mut Self) -> Option>, + ) -> Result { + let start_depth = self.search_graph().overflow_data.additional_depth; + let depth = self.search_graph().stack.len(); + while !self.search_graph().overflow_data.has_overflow(depth) { + if let Some(result) = loop_body(self) { + self.search_graph().overflow_data.additional_depth = start_depth; + return result; + } + + self.search_graph().overflow_data.additional_depth += 1; + } + self.search_graph().overflow_data.additional_depth = start_depth; + self.search_graph().overflow_data.deal_with_overflow(); + Ok(on_overflow(self)) + } +} + +impl<'tcx> OverflowHandler<'tcx> for EvalCtxt<'_, 'tcx> { + fn search_graph(&mut self) -> &mut SearchGraph<'tcx> { + &mut self.search_graph + } +} + +impl<'tcx> OverflowHandler<'tcx> for SearchGraph<'tcx> { + fn search_graph(&mut self) -> &mut SearchGraph<'tcx> { + self + } +} + impl<'tcx> SearchGraph<'tcx> { pub fn deal_with_overflow( &mut self, @@ -60,26 +96,3 @@ pub fn deal_with_overflow( response_no_constraints(tcx, goal, Certainty::Maybe(MaybeCause::Overflow)) } } - -impl<'tcx> EvalCtxt<'_, 'tcx> { - /// A `while`-loop which tracks overflow. - pub fn repeat_while_none( - &mut self, - mut overflow_body: impl FnMut(&mut Self) -> T, - mut loop_body: impl FnMut(&mut Self) -> Option>, - ) -> Result { - let start_depth = self.search_graph.overflow_data.additional_depth; - let depth = self.search_graph.stack.len(); - while !self.search_graph.overflow_data.has_overflow(depth) { - if let Some(result) = loop_body(self) { - self.search_graph.overflow_data.additional_depth = start_depth; - return result; - } - - self.search_graph.overflow_data.additional_depth += 1; - } - self.search_graph.overflow_data.additional_depth = start_depth; - self.search_graph.overflow_data.deal_with_overflow(); - Ok(overflow_body(self)) - } -} From c8dae10f14122555bfc1625b037b92eb104e67ad Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 6 Feb 2023 17:11:43 -0300 Subject: [PATCH 4/5] Check for overflow in evaluate_canonical_goal --- .../rustc_trait_selection/src/solve/mod.rs | 23 +++------- .../src/solve/search_graph/mod.rs | 43 ++++++++++++++++--- .../src/solve/search_graph/overflow.rs | 4 +- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index 358a2bcc7b9..d444ca69df1 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -211,27 +211,16 @@ fn evaluate_canonical_goal( search_graph: &'a mut search_graph::SearchGraph<'tcx>, canonical_goal: CanonicalGoal<'tcx>, ) -> QueryResult<'tcx> { - match search_graph.try_push_stack(tcx, canonical_goal) { - Ok(()) => {} - // Our goal is already on the stack, eager return. - Err(response) => return response, - } - - // We may have to repeatedly recompute the goal in case of coinductive cycles, - // check out the `cache` module for more information. + // Deal with overflow, caching, and coinduction. // - // FIXME: Similar to `evaluate_all`, this has to check for overflow. - loop { + // The actual solver logic happens in `ecx.compute_goal`. + search_graph.with_new_goal(tcx, canonical_goal, |search_graph| { let (ref infcx, goal, var_values) = tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &canonical_goal); let mut ecx = EvalCtxt { infcx, var_values, search_graph, in_projection_eq_hack: false }; - let result = ecx.compute_goal(goal); - - if search_graph.try_finalize_goal(tcx, canonical_goal, result) { - return result; - } - } + ecx.compute_goal(goal) + }) } fn make_canonical_response(&self, certainty: Certainty) -> QueryResult<'tcx> { @@ -487,7 +476,7 @@ fn evaluate_all( ) -> Result { let mut new_goals = Vec::new(); self.repeat_while_none( - |_| Certainty::Maybe(MaybeCause::Overflow), + |_| Ok(Certainty::Maybe(MaybeCause::Overflow)), |this| { let mut has_changed = Err(Certainty::Yes); for goal in goals.drain(..) { diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs index 438bcd9a7d6..9b398ef0e62 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs @@ -3,6 +3,7 @@ use self::cache::ProvisionalEntry; use super::{CanonicalGoal, Certainty, MaybeCause, QueryResult}; +use crate::solve::search_graph::overflow::OverflowHandler; use cache::ProvisionalCache; use overflow::OverflowData; use rustc_index::vec::IndexVec; @@ -13,7 +14,7 @@ pub struct StackDepth {} } -struct StackElem<'tcx> { +pub(crate) struct StackElem<'tcx> { goal: CanonicalGoal<'tcx>, has_been_used: bool, } @@ -127,7 +128,8 @@ pub(super) fn try_finalize_goal( actual_goal: CanonicalGoal<'tcx>, response: QueryResult<'tcx>, ) -> bool { - let StackElem { goal, has_been_used } = self.stack.pop().unwrap(); + let stack_elem = self.stack.pop().unwrap(); + let StackElem { goal, has_been_used } = stack_elem; assert_eq!(goal, actual_goal); let cache = &mut self.provisional_cache; @@ -156,7 +158,7 @@ pub(super) fn try_finalize_goal( self.stack.push(StackElem { goal, has_been_used: false }); false } else { - self.try_move_finished_goal_to_global_cache(tcx, &goal); + self.try_move_finished_goal_to_global_cache(tcx, stack_elem); true } } @@ -164,10 +166,11 @@ pub(super) fn try_finalize_goal( pub(super) fn try_move_finished_goal_to_global_cache( &mut self, tcx: TyCtxt<'tcx>, - goal: &CanonicalGoal<'tcx>, + stack_elem: StackElem<'tcx>, ) { + let StackElem { goal, .. } = stack_elem; let cache = &mut self.provisional_cache; - let provisional_entry_index = *cache.lookup_table.get(goal).unwrap(); + let provisional_entry_index = *cache.lookup_table.get(&goal).unwrap(); let provisional_entry = &mut cache.entries[provisional_entry_index]; let depth = provisional_entry.depth; @@ -193,4 +196,34 @@ pub(super) fn try_move_finished_goal_to_global_cache( } } } + + pub(super) fn with_new_goal( + &mut self, + tcx: TyCtxt<'tcx>, + canonical_goal: CanonicalGoal<'tcx>, + mut loop_body: impl FnMut(&mut Self) -> QueryResult<'tcx>, + ) -> QueryResult<'tcx> { + match self.try_push_stack(tcx, canonical_goal) { + Ok(()) => {} + // Our goal is already on the stack, eager return. + Err(response) => return response, + } + + self.repeat_while_none( + |this| { + let result = this.deal_with_overflow(tcx, canonical_goal); + let stack_elem = this.stack.pop().unwrap(); + this.try_move_finished_goal_to_global_cache(tcx, stack_elem); + result + }, + |this| { + let result = loop_body(this); + if this.try_finalize_goal(tcx, canonical_goal, result) { + Some(result) + } else { + None + } + }, + ) + } } diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs index 0d6863b1e81..ea62152789e 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs @@ -55,7 +55,7 @@ pub(crate) trait OverflowHandler<'tcx> { fn repeat_while_none( &mut self, - on_overflow: impl FnOnce(&mut Self) -> T, + on_overflow: impl FnOnce(&mut Self) -> Result, mut loop_body: impl FnMut(&mut Self) -> Option>, ) -> Result { let start_depth = self.search_graph().overflow_data.additional_depth; @@ -70,7 +70,7 @@ fn repeat_while_none( } self.search_graph().overflow_data.additional_depth = start_depth; self.search_graph().overflow_data.deal_with_overflow(); - Ok(on_overflow(self)) + on_overflow(self) } } From 26136c6224345743952642600f7bafe78df46449 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 14 Feb 2023 10:01:30 -0300 Subject: [PATCH 5/5] Reduce visibility of some items --- compiler/rustc_trait_selection/src/solve/mod.rs | 2 +- .../src/solve/search_graph/mod.rs | 14 +++++++------- .../src/solve/search_graph/overflow.rs | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index d444ca69df1..32fcd751b46 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -31,7 +31,7 @@ }; use rustc_span::DUMMY_SP; -use crate::solve::search_graph::overflow::OverflowHandler; +use crate::solve::search_graph::OverflowHandler; use crate::traits::ObligationCause; mod assembly; diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs index 9b398ef0e62..e9945cde5df 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs @@ -1,9 +1,9 @@ mod cache; -pub(crate) mod overflow; +mod overflow; use self::cache::ProvisionalEntry; use super::{CanonicalGoal, Certainty, MaybeCause, QueryResult}; -use crate::solve::search_graph::overflow::OverflowHandler; +pub(super) use crate::solve::search_graph::overflow::OverflowHandler; use cache::ProvisionalCache; use overflow::OverflowData; use rustc_index::vec::IndexVec; @@ -14,12 +14,12 @@ pub struct StackDepth {} } -pub(crate) struct StackElem<'tcx> { +struct StackElem<'tcx> { goal: CanonicalGoal<'tcx>, has_been_used: bool, } -pub(crate) struct SearchGraph<'tcx> { +pub(super) struct SearchGraph<'tcx> { /// The stack of goals currently being computed. /// /// An element is *deeper* in the stack if its index is *lower*. @@ -47,7 +47,7 @@ pub(super) fn is_empty(&self) -> bool { /// /// This correctly updates the provisional cache if there is a cycle. #[instrument(level = "debug", skip(self, tcx), ret)] - pub(super) fn try_push_stack( + fn try_push_stack( &mut self, tcx: TyCtxt<'tcx>, goal: CanonicalGoal<'tcx>, @@ -122,7 +122,7 @@ pub(super) fn try_push_stack( /// /// FIXME: Refer to the rustc-dev-guide entry once it exists. #[instrument(level = "debug", skip(self, tcx, actual_goal), ret)] - pub(super) fn try_finalize_goal( + fn try_finalize_goal( &mut self, tcx: TyCtxt<'tcx>, actual_goal: CanonicalGoal<'tcx>, @@ -163,7 +163,7 @@ pub(super) fn try_finalize_goal( } } - pub(super) fn try_move_finished_goal_to_global_cache( + fn try_move_finished_goal_to_global_cache( &mut self, tcx: TyCtxt<'tcx>, stack_elem: StackElem<'tcx>, diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs index ea62152789e..56409b0602b 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/overflow.rs @@ -50,7 +50,7 @@ fn deal_with_overflow(&mut self) { } } -pub(crate) trait OverflowHandler<'tcx> { +pub(in crate::solve) trait OverflowHandler<'tcx> { fn search_graph(&mut self) -> &mut SearchGraph<'tcx>; fn repeat_while_none(