Treat projections with infer as placeholder during fast reject in new solver#108830
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
compiler-errors
force-pushed
the
new-solver-fast-reject-faster
branch
5 times, most recently
from
ab25527 to
540a9a0
Compare
lcnr
left a comment
lcnr
left a comment
There was a problem hiding this comment.
As we share TreatParams with the DeepRejectCtxt, should we instead add TreatProjections separately as another argument to simplify_type and keep the current TreatParams?
that would work and is probably a bit cleaner 🤔 |
|
☔ The latest upstream changes (presumably #109019) made this pull request unmergeable. Please resolve the merge conflicts. |
compiler-errors
force-pushed
the
new-solver-fast-reject-faster
branch
2 times, most recently
from
7661cf3 to
dd430d7
Compare
compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
we should add a fixed-by-trait-solver-next label for issues like this, so that we don't forget to close them when we stabilize the new solver
compiler-errors
force-pushed
the
new-solver-fast-reject-faster
branch
from
dd430d7 to
84d254e
Compare
|
@bors r=lcnr |
bors
added
S-waiting-on-bors
…ject-faster, r=lcnr Treat projections with infer as placeholder during fast reject in new solver r? `@lcnr` Kind of a shame that we need to change all of the call sites for `for_each_relevant_impl`, etc. to pass an extra parameter. I guess I could have the "default" fn which calls a configurable fn?
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#108419 (Stabilize `atomic_as_ptr`) - rust-lang#108507 (use `as_ptr` to determine the address of atomics) - rust-lang#108607 (Don't use fd-lock on Solaris in bootstrap) - rust-lang#108830 (Treat projections with infer as placeholder during fast reject in new solver) - rust-lang#109055 (create `config::tests::detect_src_and_out` test for bootstrap) - rust-lang#109058 (Document BinOp::is_checkable) - rust-lang#109081 (simd-wide-sum test: adapt for LLVM 17 codegen change) - rust-lang#109083 (Update books) - rust-lang#109088 (Gracefully handle `#[target_feature]` on statics) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
This caused a perf regression (see results). @lcnr / @compiler-errors do we have a sense of whether this was expected or not? Any thoughts on possible fixes? |
|
ahh, that code is really hot so this small change has such a big impact 🤔 we should maybe change |
|
I can take a look at doing that unless lcnr beats me to it |
|
I am surprised by this regression. I thought the fast reject code was very important for |
…ct-faster-2, r=lcnr Don't pass `TreatProjections` separately to `fast_reject` Don't pass `TreatProjections` separately to `fast_reject`, and instead use the original approach of switching on two variants of `TreatParams` (undoes this: rust-lang#108830 (review)). Fixes the regression introduced in rust-lang#108830 (comment)
…-2, r=lcnr Don't pass `TreatProjections` separately to `fast_reject` Don't pass `TreatProjections` separately to `fast_reject`, and instead use the original approach of switching on two variants of `TreatParams` (undoes this: rust-lang/rust#108830 (review)). Fixes the regression introduced in rust-lang/rust#108830 (comment)
6 participants
r? @lcnr
Kind of a shame that we need to change all of the call sites for
for_each_relevant_impl, etc. to pass an extra parameter. I guess I could have the "default" fn which calls a configurable fn?