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

Add the generic_associated_types_extended feature #94869

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

jackh726
Copy link
Member

Right now, this only ignore obligations that reference new placeholders in poly_project_and_unify_type. In the future, this might do other things, like allowing object-safe GATs.

This feature is incomplete and quite likely unsound. This is mostly just for testing out potential future APIs using a "relaxed" set of rules until we figure out proper rules.

Also drive by cleanup of adding a ProjectAndUnifyResult enum instead of using a Result<Result<Option>>.

r? @nikomatsakis

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

bors commented Mar 25, 2022

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

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me with the comment I asked for (or not, up to you).

.filter(|obligation| {
let mut visitor = MaxUniverse::new();
obligation.predicate.visit_with(&mut visitor);
visitor.max_universe() < new_universe
Copy link
Member

Choose a reason for hiding this comment

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

Can you leave a comment:

  1. saying that this is temporary/hack (or whatever you want to call it), and
  2. explaining what this "means" -- i.e. that this filters predicates referencing the newly replaced placeholder regions

Also, dumb question, what's the difference between visitor.max_universe() < new_universe and visitor.max_universe() <= old_universe? I guess it probably doesn't change behavior here.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, the new_universe here refers to the universe we just created (from the replace_bound_vars_with_placeholders). Importantly, we want to skip any obligations containing this (so <= doesn't work, because = means it contains this new_universe). Any obligations with a max universe greater than new_universe must have been created from a nested HRTB (so maybe from with a fn). These are equally weird.

visitor.max_universe() < new_universe should be the same as visitor.max_universe() <= old_universe though, since new_universe should be old_universe+1, but that's more confusing IMO.

@jackh726
Copy link
Member Author

Updated with a comment. @bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Mar 30, 2022

📌 Commit 4f36c94 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 Mar 30, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…errors

Add the generic_associated_types_extended feature

Right now, this only ignore obligations that reference new placeholders in `poly_project_and_unify_type`. In the future, this might do other things, like allowing object-safe GATs.

**This feature is *incomplete* and quite likely unsound. This is mostly just for testing out potential future APIs using a "relaxed" set of rules until we figure out *proper* rules.**

Also drive by cleanup of adding a `ProjectAndUnifyResult` enum instead of using a `Result<Result<Option>>`.

r? `@nikomatsakis`
@Dylan-DPC
Copy link
Member

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 Mar 30, 2022
@jackh726
Copy link
Member Author

Darn NLL

@jackh726
Copy link
Member Author

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Mar 30, 2022

📌 Commit 4e570a6 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 Mar 30, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 31, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#93901 (Stabilize native library modifier syntax and the `whole-archive` modifier specifically)
 - rust-lang#94806 (Fix `cargo run tidy`)
 - rust-lang#94869 (Add the generic_associated_types_extended feature)
 - rust-lang#95011 (async: Give predictable name to binding generated from .await expressions.)
 - rust-lang#95251 (Reduce max hash in raw strings from u16 to u8)
 - rust-lang#95298 (Fix double drop of allocator in IntoIter impl of Vec)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e08ab08 into rust-lang:master Mar 31, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 31, 2022
@jackh726 jackh726 deleted the gats_extended branch March 31, 2022 03:49
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 1, 2022
…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`
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 2, 2022
…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`
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 7, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#93901 (Stabilize native library modifier syntax and the `whole-archive` modifier specifically)
 - rust-lang#94806 (Fix `cargo run tidy`)
 - rust-lang#94869 (Add the generic_associated_types_extended feature)
 - rust-lang#95011 (async: Give predictable name to binding generated from .await expressions.)
 - rust-lang#95251 (Reduce max hash in raw strings from u16 to u8)
 - rust-lang#95298 (Fix double drop of allocator in IntoIter impl of Vec)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
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.

7 participants