Skip to content
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

Fix RPITITs in default trait methods (by assuming projection predicates in param-env) #108203

Merged
merged 3 commits into from
Feb 19, 2023

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Feb 18, 2023

Instead of having special projection logic that allows us to turn ProjectionTy(RPITIT, [Self#0, ...]) into OpaqueTy(RPITIT, [Self#0, ...]), we can instead augment the param-env of default trait method bodies to assume these as projection predicates. This should allow us to only project where we're allowed to!

In order to make this work without introducing a bunch of cycle errors, we additionally tweak the OpaqueTypeExpander used by ParamEnv::with_reveal_all_normalized to not normalize the right-hand side of projection predicates. This should be fine, because if we use the projection predicate to normalize some other projection type, we'll continue to normalize the opaque that it gets projected to.

This also makes it possible to support default trait methods with RPITITs in an associated-type based RPITIT lowering strategy without too much extra effort.

Fixes #107002
Alternative to #108142

@rustbot
Copy link
Collaborator

rustbot commented Feb 18, 2023

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 18, 2023
@compiler-errors
Copy link
Member Author

r? types

@jackh726
Copy link
Member

I like this. Not sure if you had anyone else in mind that you wanted to look at this?

r=me if not

@compiler-errors
Copy link
Member Author

I don't think so, I already discussed the idea with @lcnr and it seems to have worked. We can always remove it later.

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Feb 18, 2023

📌 Commit e0f41ed8597680bcdf833486f43c1b4ced755ec6 has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 18, 2023
@compiler-errors
Copy link
Member Author

@bors r- conflict oops

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 18, 2023
@compiler-errors
Copy link
Member Author

@bors r=jackh726

@bors
Copy link
Contributor

bors commented Feb 18, 2023

📌 Commit 3e57b20 has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 18, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2023
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#107766 (Fix json reexports of different items with same name)
 - rust-lang#108129 (Correctly handle links starting with whitespace)
 - rust-lang#108188 (Change src/etc/vscode_settings.json to always treat ./library as the sysroot source)
 - rust-lang#108203 (Fix RPITITs in default trait methods (by assuming projection predicates in param-env))
 - rust-lang#108212 (Download rustfmt regardless of rustc being set in config.toml)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d2aef58 into rust-lang:master Feb 19, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 19, 2023
@fasterthanlime
Copy link
Contributor

This did the trick for hring, thanks for the fix!

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 21, 2023
…lt-constraint, r=oli-obk

Add a test for default trait method with RPITITs

This didn't work in rust-lang#107013, but now that rust-lang#108203 has landed, let's make sure we don't regress it.

r? types
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2023
…hod-with-nested-rpitits, r=spastorino

Account for binders correctly when adding default RPITIT method assumption

As of rust-lang#108203, we install extra projection predicates into the param-env of a default trait method when it has return-position `impl Trait` (or is async).

The implementation didn't account for the fact that it's walking into and out of binders, so we just need to shift all the debruijn indices accordingly when constructing the projection predicates.

Fixes rust-lang#108579

r? types
@compiler-errors compiler-errors deleted the rpitit-fix-defaults-2 branch August 11, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected behaviour when calling associated async function of a trait with default implementations
6 participants