Repository navigation
Conversation
rustbot
added
the
S-waiting-on-review
| self.obligations.register(obligation, stalled_on); | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
Hmm, could we just mirror the following lines?
https://github.com/rust-lang/rust/blob/f8125e48dc669320b4e5e83487dcbfb681957961/compiler/rustc_trait_selection/src/solve/fulfill.rs#L172-L178
There was a problem hiding this comment.
Might not be possible b/c we haven't updated the solver for a while. Maybe blocked on updating it.
There was a problem hiding this comment.
An update isn't needed, but a bump of solver to 0.175 would make it cleaner.
There was a problem hiding this comment.
@ada4a has been working on the solver update but it would take some time.
cc https://rust-lang.zulipchat.com/#narrow/channel/185405-t-compiler.2Frust-analyzer/topic/Updating.20next-solver/with/629296217
So, if we could bump it to 0.175 without breaking things that would be nice, but if not, working with the current solver would make sense, I think
There was a problem hiding this comment.
(I'm slowly doing the updates locally, and am now at 0.169. But right now I'm on vacation until the end of the week + we need to set up trusted publishing for the shared crates to actually perform the updates. Every version bump is a gamble in terms of how much fixing-up is required, so it's hard to estimate when I'll be able to get to 0.175 – maybe middle/end of November?)
There was a problem hiding this comment.
Sorry for disturbing your vacation 😅 Let me know if there's anything I could help a bit and have a good time!
There was a problem hiding this comment.
All good, just wanted to give some context^^
You're involved with the next solver as well, right? If I have any questions while doing the update, I might ping you on Zulip :3 (probably in the thread you've linked)
Thank you, you too! Talk to you soon:)
There was a problem hiding this comment.
Would it be an option to merge this fix as is, but also add a FIXME including the simpler fix you're talking about @Hexay? That way, I could switch over to it once I get to 0.175. In case getting this fix in quickly is a priority
|
For reference, the impact on real-world workloads is very real. Using rust-lang/rustc-perf#2601 from rust-lang/rust#159933, the (AI assisted) benchmarks yield: Compared PR head Median inference time, three runs per variant:
The improvement is substantial, but scaling remains approximately quadratic: N=4000 → N=8000 costs 4.02× for patched These are batch-analysis results with build scripts and procedural macros disabled. Slint’s whole-process speedup was 1.41×; incremental editor behavior was not measured |
|
I object to any change to the next solver until we sync or unless @ada4a specifically approves it. I also object specifically to this one since upstream fulfillment is already shared between rustc and r-a. |
I frankly still lack the knowledge to make a decision, so I'll trust your judgement.
Just for my understanding: by this you mean that we copy-paste rustc code here, right? AFAICT the code comes from https://github.com/rust-lang/rust/blob/f01f1dda4a792e18fe1a2ae6a5f205cc2b9d3c97/compiler/rustc_trait_selection/src/solve/fulfill.rs, and |
It appears I was wrong, that PR (rust-lang/rust#160485) isn't merged yet. It's still about to be. |
|
Ohh, exciting:)) I kinda wonder whether that makes it more okay to make some temporary changes to our fulfill.rs (like in this PR), given that we'll delete the whole file anyway once rust-lang/rust#160485 lands. But again, no preference either way. |
Goals that stalled on an earlier pass build new
SearchGraphandEvalCtxt, before reaching the stall check.No divergence found across large dependency workspace.