-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
propagate opaque types to the typeck root #135445
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #135465) made this pull request unmergeable. Please resolve the merge conflicts. |
f8b46e1
to
3d6168a
Compare
alright, there are still a few hacks and bad diagnostics, but it's otherwise working as intended. Going to do a crater run to check whether it's breaking anything. @bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
propagate opaque types to the typeck root still far from properly working r? `@compiler-errors`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I am not current with the state of the new solver, but no matter how I think about it, I find it difficult to justify the added complexity (edit: of this PR, not the solver :). Member constraints are not simple post-inference checks—they directly impact region inference, and simply propagating them to the parent function renders them ineffective. Is the goal here to allow defining opaque types with external lifetime arguments inside closures? If so, this approach does not work once the hidden type contains lifetimes struct Relate<T>(T, T);
fn test<'a>() -> impl Sized + 'a {
|| Relate("", test::<'a>()); // Opaque<'a> := &'empty str
"" // Opaque<'a> := &'a str
} |
This also works with this approach as we lift The only pattern I am aware of which is not supported by this approach is the following (regardless of whether we're in a closure) struct Foo;
impl Foo {
fn recur<'a>(&'a self, b: bool) -> impl Sized + 'a {
if b {
let temp = Foo;
// temp.recur(borrow, false);
let borrow = &temp;
Foo::recur(borrow, false); // defines in new solver, opaque use in old
}
}
} And that's more a general issue of this recursive call being a defining one. Fixing this one requires a different perspective on opaques to allow non-defining uses in the defining scope |
So hidden type regions are lifted as well. Interesting indeed. 👍 This puts a heavy strain on the "region constraint propagation" code, which has a poor track record and, in some cases, is only approximate, as you encountered in #104477. |
☔ The latest upstream changes (presumably #135615) made this pull request unmergeable. Please resolve the merge conflicts. |
995a306
to
417be2c
Compare
[DO NOT MERGE] bootstrap with `-Znext-solver=globally` A revival of rust-lang#124812. Current status: ~~`./x.py b --stage 2` passes 🎉~~ `try` builds succeed 🎉 🎉 🎉 [first perf run](rust-lang#133502 (comment)) 👻 ### in-flight changes - ce66d92 is a rebased version of rust-lang#125334, unsure whether I actually want to land this PR for now - rust-lang#135445 - https://github.com/lcnr/rust/tree/opaque-type-method-call support method calls on not-yet defined opaque types in their defining scope ☠️ r? `@ghost`
The job Click to see the possible cause of the failure (guessed by this bot)
|
@rustbot author since this is still draft/being worked on i think? |
☔ The latest upstream changes (presumably #136085) made this pull request unmergeable. Please resolve the merge conflicts. |
still far from properly working
r? @compiler-errors