Skip to content

Make GATs object safe under generic_associated_types_extended feature#94911

Merged
bors merged 1 commit intorust-lang:masterfrom
jackh726:gats_extended_2
Apr 2, 2022
Merged

Make GATs object safe under generic_associated_types_extended feature#94911
bors merged 1 commit intorust-lang:masterfrom
jackh726:gats_extended_2

Conversation

@jackh726
Copy link
Member

@jackh726 jackh726 commented Mar 13, 2022

Based on #94869

Let's say we have

trait StreamingIterator {
    type Item<'a> where Self: 'a;
}

And dyn for<'a> StreamingIterator<Item<'a> = &'a i32>.

If we ask (dyn for<'a> StreamingIterator<Item<'a> = &'a i32>): StreamingIterator, then we have to prove that for<'x> (&'x i32): Sized. So, we generate new bound vars to subst for the GAT generics.

Importantly, this doesn't fully verify that these are usable and sound.

r? @nikomatsakis

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 13, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 13, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented Mar 25, 2022

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

@jackh726 jackh726 mentioned this pull request Mar 29, 2022
Closed
@rust-log-analyzer

This comment has been minimized.

Sign in to view
compiler-errors
compiler-errors approved these changes Mar 30, 2022
View reviewed changes
Copy link
Contributor

@compiler-errors compiler-errors left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes look good -- the logic you're using to create new bound vars for the GATs is inspired from #87900, right?

r=me once the parent PR lands (and you rebase that first commit out)

compiler/rustc_trait_selection/src/traits/object_safety.rs Outdated
Copy link
Contributor

@compiler-errors compiler-errors Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say that this should also deny GATs with type parameters, but it seems we already disallow those elsewhere: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1af97ded49aa74d43f72f0c3b70658e9

@bors
Copy link
Collaborator

bors commented Mar 31, 2022

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

@jackh726
Copy link
Member Author

jackh726 commented Apr 1, 2022

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Apr 1, 2022

📌 Commit d3fe28b has been approved by compiler-errors

@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 Apr 1, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 1, 2022
@matthiaskrgr
Rollup merge of rust-lang#94911 - jackh726:gats_extended_2, r=compile…
bd70a56 
…r-errors

Make GATs object safe under generic_associated_types_extended feature

Based on rust-lang#94869

Let's say we have
```rust
trait StreamingIterator {
    type Item<'a> where Self: 'a;
}
```
And `dyn for<'a> StreamingIterator<Item<'a> = &'a i32>`.

If we ask `(dyn for<'a> StreamingIterator<Item<'a> = &'a i32>): StreamingIterator`, then we have to prove that `for<'x> (&'x i32): Sized`. So, we generate *new* bound vars to subst for the GAT generics.

Importantly, this doesn't fully verify that these are usable and sound.

r? `@nikomatsakis`
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 1, 2022
Closed
@Dylan-DPC
Copy link
Member

Dylan-DPC commented Apr 2, 2022

failed in rollup

@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 Apr 2, 2022
@compiler-errors
Copy link
Contributor

compiler-errors commented Apr 2, 2022

NLL strikes again...

@jackh726
Copy link
Member Author

jackh726 commented Apr 2, 2022

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Apr 2, 2022

📌 Commit 52b00db has been approved by compiler-errors

@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 Apr 2, 2022
@bors
Copy link
Collaborator

bors commented Apr 2, 2022

⌛ Testing commit 52b00db with merge 8f96ef4...

@bors
Copy link
Collaborator

bors commented Apr 2, 2022

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 8f96ef4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 2, 2022
@bors bors merged commit 8f96ef4 into rust-lang:master Apr 2, 2022
@rustbot rustbot added this to the 1.61.0 milestone Apr 2, 2022
@jackh726 jackh726 deleted the gats_extended_2 branch April 2, 2022 21:25
@rust-timer
Copy link
Collaborator

rust-timer commented Apr 2, 2022

Finished benchmarking commit (8f96ef4): comparison url.

Summary:

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: no relevant changes found. 1 results were found to be statistically significant but too small to be relevant.
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 1 0 0 0
mean2 N/A 0.2% N/A N/A NaN%
max N/A 0.2% N/A N/A 0.0%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

@matthiaskrgr matthiaskrgr mentioned this pull request Aug 3, 2023
Closed
@fmease fmease added the F-generic_associated_types_extended `#![feature(generic_associated_types_extended)]` label Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@compiler-errors compiler-errors compiler-errors approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nikomatsakis nikomatsakis

Labels

F-generic_associated_types_extended `#![feature(generic_associated_types_extended)]` merged-by-bors This PR was explicitly merged by bors. 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

Milestone

1.61.0

Development

Successfully merging this pull request may close these issues.

10 participants

@jackh726 @rust-log-analyzer @bors @Dylan-DPC @compiler-errors @rust-timer @nikomatsakis @rust-highfive @fmease @rustbot