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

Report change in RPITIT lifetime capture clauses. #105258

Closed
wants to merge 1 commit into from

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Dec 4, 2022

Forbid RPITIT from implementations from capturing more lifetimes than the trait definitions allows.

@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2022

r? @oli-obk

(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 Dec 4, 2022
@rust-log-analyzer

This comment was marked as outdated.

@cjgillot cjgillot force-pushed the rpit-mismatch branch 2 times, most recently from 56de728 to 2c08b8e Compare December 4, 2022 15:28
@rust-log-analyzer

This comment has been minimized.

@cjgillot cjgillot 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2022
@cjgillot cjgillot marked this pull request as draft December 4, 2022 16:31
@cjgillot cjgillot marked this pull request as ready for review December 4, 2022 19:43
@cjgillot cjgillot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Dec 5, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Dec 5, 2022

📌 Commit 503bc7c has been approved by oli-obk

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 Dec 5, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2022
Report change in RPITIT lifetime capture clauses.

Forbid RPITIT from implementations from capturing more lifetimes than the trait definitions allows.
@matthiaskrgr
Copy link
Member

I think this failed in a rollup, not certain though
#105327 (comment)

@cjgillot
Copy link
Contributor Author

cjgillot commented Dec 6, 2022

Indeed, it's buggy.
@bors r-

@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 Dec 6, 2022
@cjgillot cjgillot marked this pull request as draft December 10, 2022 07:47
@bors
Copy link
Contributor

bors commented Dec 17, 2022

☔ The latest upstream changes (presumably #105804) made this pull request unmergeable. Please resolve the merge conflicts.

rust-cloud-vms bot pushed a commit to compiler-errors/rust that referenced this pull request Jun 30, 2023
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Jul 1, 2023
…ures, r=oli-obk

Error when RPITITs' hidden types capture more lifetimes than their trait definitions

This implements a stricter set of captures rules for RPITITs. They now may only capture:
1. Lifetimes from the impl header (both the self type and any trait substs -- we may want to restrict just to the self type's lifetimes, but the PR makes that easy to do, too)
2. Lifetimes mentioned by the `impl Trait` in the trait method's definition.

Namely, they may not mention lifetimes from the method (early or late) that are not mentioned in the `impl Trait`.

cc rust-lang#105258 which I think was trying to do this too, though I'm not super familiar with what exactly differs from that or why that one was broken.
cc rust-lang#112194 (doesn't fix this issue per se, because it's still an open question, but I think this is objectively better than the status quo, and gets us closer to resolving that issue.)

Technically is a fix for the ICE in rust-lang#108580, but it turns that issue into an error now. We can decide separately whether or not nested RPITITs should capture lifetimes from their parents.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 1, 2023
…ures, r=oli-obk

Error when RPITITs' hidden types capture more lifetimes than their trait definitions

This implements a stricter set of captures rules for RPITITs. They now may only capture:
1. Lifetimes from the impl header (both the self type and any trait substs -- we may want to restrict just to the self type's lifetimes, but the PR makes that easy to do, too)
2. Lifetimes mentioned by the `impl Trait` in the trait method's definition.

Namely, they may not mention lifetimes from the method (early or late) that are not mentioned in the `impl Trait`.

cc rust-lang#105258 which I think was trying to do this too, though I'm not super familiar with what exactly differs from that or why that one was broken.
cc rust-lang#112194 (doesn't fix this issue per se, because it's still an open question, but I think this is objectively better than the status quo, and gets us closer to resolving that issue.)

Technically is a fix for the ICE in rust-lang#108580, but it turns that issue into an error now. We can decide separately whether or not nested RPITITs should capture lifetimes from their parents.

r? ``@oli-obk``
@cjgillot cjgillot closed this Sep 23, 2023
@cjgillot cjgillot deleted the rpit-mismatch branch September 23, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

6 participants