Show the nested goal chain for the next solver overflow FCW - #161341
Show the nested goal chain for the next solver overflow FCW#161341adwinwhite wants to merge 5 commits into
Conversation
| @@ -344,6 +344,10 @@ pub struct InferCtxt<'tcx> { | |||
| /// checking so we disable the FCW there as well. | |||
| enable_next_solver_overflow_fcw: bool, | |||
There was a problem hiding this comment.
change this to a Cell instead? 🤔
| { | ||
| self.in_next_solver_overflow_fcw.set(true); | ||
| let ret = f(); | ||
| self.in_next_solver_overflow_fcw.set(false); |
There was a problem hiding this comment.
feels safer to replace previous entry and then revert to that one in functions like this, otherwise nesting it twice breaks the outer call
858bfc1 to
0d21359
Compare
This comment has been minimized.
This comment has been minimized.
0d21359 to
61aecad
Compare
61aecad to
ce9f31f
Compare
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
|
☔ The latest upstream changes (presumably #161043) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
|
|
||
| /// We need to disable the fcw if we're already in a fcw emitting to avoid | ||
| /// indefinite triggering. | ||
| pub fn enter_next_solver_overflow_fcw<F, R>(&self, mut f: F) -> R |
There was a problem hiding this comment.
| pub fn enter_next_solver_overflow_fcw<F, R>(&self, mut f: F) -> R | |
| pub fn with_disabled_next_solver_overflow_fcw<F, R>(&self, mut f: F) -> R |
maybe? the current name feels a bit unclear to me
| nested_goals: &evaluation_result.nested_goals, | ||
| min_reachable_available_depth: AvailableDepth( | ||
| available_depth.0 - evaluation_result.required_depth, | ||
| available_depth.0 - evaluation_result.required_depth.0, |
There was a problem hiding this comment.
impl Sub<RequiredDepth> for AvailableDepth and use it in these 2 places
| = help: consider increasing the recursion limit by adding a `#![recursion_limit = "16"]` attribute to your crate (`fcw_on_normalization`) | ||
| = help: or consider adding a manual `impl` of auto traits like `Send` for intermediate types, if auto traits are involved | ||
| = note: which requires <W<W<W<W<W<W<W<W<W<()>>>>>>>>> as HasAssoc>::Assoc == () | ||
| = note: which requires <W<W<W<W<W<W<W<W<()>>>>>>>> as HasAssoc>::Assoc == () |
There was a problem hiding this comment.
why does this stop after 2 goals? that's surprising
| } | ||
|
|
||
| fn visit_goal(&mut self, goal: &inspect::InspectGoal<'_, 'tcx>) -> Self::Result { | ||
| self.predicates.push(goal.goal().predicate); |
There was a problem hiding this comment.
does this currently print both NormalizesTo and the Projection goal? if so, we should probably filter the NormalizesTo goal here :>
Previously we only showed a single root goal for the FCW. It was difficult to find out how the goal overflowed.
We display a proving chain now which should help users identify relevant types or auto traits.
This will affect perf for crates emitting the FCW. E.g.
calimero-storegoes from 4.7s -> 5.7s in local testing since it emits thousands of FCWs internally. The FCW is a mitigation of future hard error and authors are expected to resolve it so it's probably acceptable. It doesn't affect crates without the FCW.r? lcnr