Make DefiningAnchor::Bind only store the opaque types that may be constrained, instead of the current infcx root item.
#121796
Conversation
rustbot
added
S-waiting-on-review
oli-obk
force-pushed
the
eager_opaque_checks3
branch
from
f09d6ba
to
d72008e
Compare
compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
Show resolved
Hide resolved
|
Note that the |
compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
rustbot
added
S-blocked
oli-obk
force-pushed
the
eager_opaque_checks3
branch
from
d72008e
to
73090ab
Compare
This comment was marked as resolved.
This comment was marked as resolved.
oli-obk
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #121462) made this pull request unmergeable. Please resolve the merge conflicts. |
oli-obk
force-pushed
the
eager_opaque_checks3
branch
2 times, most recently
from
fbcfe97
to
04461ee
Compare
|
@bors try @rust-timer queue (would be fine with landing this even if it's perf negative, i'm just curious) |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
Make `DefiningAnchor::Bind` only store the opaque types that may be constrained, instead of the current infcx root item. This makes `Bind` almost always be empty, so we can start forwarding it to queries, allowing us to remove `Bubble` entirely (not done in this PR) The only behaviour change is in diagnostics. r? `@lcnr` `@compiler-errors`
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (ee25694): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 644.654s -> 647.067s (0.37%) |
rustbot
added
perf-regression
|
The regression in full builds is entirely due to all the invocations of opaque_types_defined_by Incremental builds regress due to depgraph encoding. I have ideas for the incremental cases, not sure what to do about the extra work in full builds |
bors
added
the
S-waiting-on-author
…onstrained, instead of the current infcx root item. This makes `Bind` almost always be empty, so we can start forwarding it to queries, allowing us to remove `Bubble` entirely
…ningAnchor::Bind` with an empty list
oli-obk
force-pushed
the
eager_opaque_checks3
branch
from
1eee039
to
b0328c2
Compare
|
@bors r=lcnr |
bors
added
S-waiting-on-bors
Pass list of defineable opaque types into canonical queries based on rust-lang#121796 This eliminates `DefiningAnchor::Bubble` for good and brings the old solver closer to the new one wrt cycles and nested obligations. At that point the difference between `DefiningAnchor::Bind([])` and `DefiningAnchor::Error` was academic. We only used the difference for some sanity checks, which actually had to be worked around in places, so I just removed `DefiningAnchor` entirely and just stored the list of opaques that may be defined. fixes rust-lang#108498 * [ ] run crater
|
☀️ Test successful - checks-actions |
1 similar comment
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (4ccbb7d): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 669.92s -> 671.027s (0.17%) |
Strip placeholders from hidden types before remapping generic parameter When remapping generic parameters in the hidden type to the generic parameters of the definition of the opaque, we assume that placeholders cannot exist. Instead of just patching that site, I decided to handle it earlier, directly in `infer_opaque_types`, where we are already doing all the careful lifetime handling. fixes rust-lang#122694 the reason that ICE now occurred was that we stopped treating `operation` as being in the defining scope, so the TAIT became part of the hidden type of the `async fn`'s opaque type instead of just bailing out as ambiguos I think ```rust use std::future::Future; mod foo { type FutNothing<'a> = impl 'a + Future<Output = ()>; //~^ ERROR: unconstrained opaque type } async fn operation(_: &mut ()) -> () { //~^ ERROR: concrete type differs from previous call(operation).await //~^ ERROR: concrete type differs from previous } async fn call<F>(_f: F) where for<'any> F: FnMut(&'any mut ()) -> foo::FutNothing<'any>, { //~^ ERROR: expected generic lifetime parameter, found `'any` } ``` would have already had the same ICE before rust-lang#121796
Strip placeholders from hidden types before remapping generic parameter When remapping generic parameters in the hidden type to the generic parameters of the definition of the opaque, we assume that placeholders cannot exist. Instead of just patching that site, I decided to handle it earlier, directly in `infer_opaque_types`, where we are already doing all the careful lifetime handling. fixes rust-lang#122694 the reason that ICE now occurred was that we stopped treating `operation` as being in the defining scope, so the TAIT became part of the hidden type of the `async fn`'s opaque type instead of just bailing out as ambiguos I think ```rust use std::future::Future; mod foo { type FutNothing<'a> = impl 'a + Future<Output = ()>; //~^ ERROR: unconstrained opaque type } async fn operation(_: &mut ()) -> () { //~^ ERROR: concrete type differs from previous call(operation).await //~^ ERROR: concrete type differs from previous } async fn call<F>(_f: F) where for<'any> F: FnMut(&'any mut ()) -> foo::FutNothing<'any>, { //~^ ERROR: expected generic lifetime parameter, found `'any` } ``` would have already had the same ICE before rust-lang#121796
Rollup merge of rust-lang#122733 - oli-obk:error_prop, r=compiler-errors Strip placeholders from hidden types before remapping generic parameter When remapping generic parameters in the hidden type to the generic parameters of the definition of the opaque, we assume that placeholders cannot exist. Instead of just patching that site, I decided to handle it earlier, directly in `infer_opaque_types`, where we are already doing all the careful lifetime handling. fixes rust-lang#122694 the reason that ICE now occurred was that we stopped treating `operation` as being in the defining scope, so the TAIT became part of the hidden type of the `async fn`'s opaque type instead of just bailing out as ambiguos I think ```rust use std::future::Future; mod foo { type FutNothing<'a> = impl 'a + Future<Output = ()>; //~^ ERROR: unconstrained opaque type } async fn operation(_: &mut ()) -> () { //~^ ERROR: concrete type differs from previous call(operation).await //~^ ERROR: concrete type differs from previous } async fn call<F>(_f: F) where for<'any> F: FnMut(&'any mut ()) -> foo::FutNothing<'any>, { //~^ ERROR: expected generic lifetime parameter, found `'any` } ``` would have already had the same ICE before rust-lang#121796
This makes
Bindalmost always be empty, so we can start forwarding it to queries, allowing us to removeBubbleentirely (not done in this PR)The only behaviour change is in diagnostics.
r? @lcnr @compiler-errors