Implement should_continue in chalk-recursive #774
Conversation
|
Build failed because of #775 |
There was a problem hiding this comment.
So, returning NoSolution when stopping because should_continue returns false is inaccurate.
This should maybe be refactored a bit, but None means we know there is no solution. Ok(Ambig) means "there might be solution or there might not be, we don't know". Stopping solving early falls under that.
Depending on how helpful this really is, I might be okay with landing this with really strong comments to make the above limitation clear.
|
Alright, changed it to |
| @@ -17,7 +17,7 @@ pub trait AggregateOps<I: Interner> { | |||
| &self, | |||
| root_goal: &UCanonical<InEnvironment<Goal<I>>>, | |||
| answers: impl context::AnswerStream<I>, | |||
| should_continue: impl std::ops::Fn() -> bool, | |||
| should_continue: impl std::ops::Fn() -> bool + Clone, | |||
There was a problem hiding this comment.
Do you need this clone bound? You should be able to pass in &should_continue to any functions to avoid a move
There was a problem hiding this comment.
I'll add a comment so the workaround can be removed when that's fixed.
yeah probably @bors r+ |
|
☀️ Test successful - checks-actions |
Update to Chalk 88 This Chalk release introduces fuel for the recursive solver ([chalk#774](rust-lang/chalk#774)). I'm not sure how often it calls `should_continue` compared to the other solver, so we might want to increase `CHALK_SOLVER_FUEL`, the current default value of 100 might be too low. This should fix a lot of hangs and crashes, for example this solves the hang in #12897.
This just returns
NoSolutionif it shouldn't continue, but that should already be useful to rust-analyzer.Note: Cloning of
should_continueis a workaround to a rustc bug (#95734)