Disambiguate run_if under rustc's nex generation trait solver - #25533
Disambiguate run_if under rustc's nex generation trait solver#25533HugoPeters1024 wants to merge 1 commit into
Conversation
|
FYI this is tracked in #25511, it seems the issue is not really an ambiguity but rather an overflow computing some Could also you check how the compile times compare when stacked on top of #25512 ? |
|
Thanks @SkiFire13! I should've done a better job scouring existing issues. I haphazardly assumed that I found myself in some niche rabbit hole that wouldn't have been explored before, glad to hear that's not the case!
This actually gives very revealing results! Your patch by itself also fixes the same underlying issue that my PR was attempting to fix, and so does compile with the next generation solver. Adding my patch then naturally doesn't make a big difference, I can only assume that the differences are attributable to noise.
|
Can you run the "25512 only + Next" and "run_if + 25512 + Next" a couple more times? I'd say 1s total build time for 1 line (maybe with a comment why it was added, i.e., to reduce comp times) would be a nice win, should it turn out not to be noise :) |
Objective
rustcwill soon stabilize a next generation trait solver: rust-lang/goals#113.To use the new solver (and reproduce the compiler error), run the following:
It turns out that this solver is less equipped to disambiguate in some cases, which affect
bevy_pbrin a few occasions.Solution
Some generic function items implement both
IntoScheduleConfigsandObserverSystemExt. Both traits providerun_if, so the next solver correctlyreports the method call as ambiguous.
Convert the function item to
ScheduleConfigsbefore adding its run condition:create_surfaces .into_configs() .run_if(need_surface_configuration)ScheduleConfigs::run_ifis then unambiguous.Testing
Tested on macOS/aarch64 with separate clean target directories. Looks like this solver is going to shave off meaningful time of bevy's slowest crate 🥳
bevy_pbrtotal-Znext-solver=coherence)-Znext-solver=globally)Open question
I suppose this is technically a regression in
rustc, although possibly a reasonable one. I can imagine that the disambiguation process was part of the reason that the old solver was less performant. That said, I'll still report this upstream for visiblity.