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

Ban associated type bounds in bad positions #108063

Merged

Conversation

compiler-errors
Copy link
Member

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

We should not try to lower associated type bounds into TAITs in positions where impl Trait is not allowed (except for in where clauses, like where T: Trait<Assoc: Bound>).

This is achieved by using the same rustc_ast_lowering machinery as impl-trait does to characterize positions as universal/existential/disallowed.

Fixes #106077

Split out the first commit into #108066, since it's not really related.

@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2023

r? @eholk

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

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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 14, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2023

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

@compiler-errors compiler-errors force-pushed the associated-type-bounds-in-bad-position branch from ff71676 to ec3b723 Compare February 14, 2023 23:03
@bors
Copy link
Contributor

bors commented Feb 16, 2023

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

@compiler-errors compiler-errors force-pushed the associated-type-bounds-in-bad-position branch 2 times, most recently from 4b16922 to 13ed5df Compare February 18, 2023 20:19
Copy link
Contributor

@eholk eholk left a comment

Choose a reason for hiding this comment

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

Looks pretty good! Just a few questions and wording suggestions.

compiler/rustc_ast_lowering/src/lib.rs Outdated Show resolved Hide resolved
tests/ui/associated-type-bounds/inside-adt.stderr Outdated Show resolved Hide resolved
@compiler-errors compiler-errors force-pushed the associated-type-bounds-in-bad-position branch from 313e896 to 08add30 Compare February 22, 2023 01:11
@bors
Copy link
Contributor

bors commented Feb 22, 2023

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

@compiler-errors compiler-errors force-pushed the associated-type-bounds-in-bad-position branch from 08add30 to b14eb0c Compare February 22, 2023 21:52
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

@bors r+

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 22, 2023

📌 Commit b14eb0c has been approved by cjgillot

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 22, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#108063 (Ban associated type bounds in bad positions)
 - rust-lang#108208 (Correctly handle aggregates in DataflowConstProp)
 - rust-lang#108218 (Windows: Quote more batch file arguments)
 - rust-lang#108349 (rustdoc: Prevent duplicated imports)
 - rust-lang#108350 (Use associated type bounds in some places in the compiler)
 - rust-lang#108358 (Add git config command to `.git-blame-ignore-revs`)
 - rust-lang#108373 (hir-analysis: make where-clause-on-main diagnostic translatable)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ef27e43 into rust-lang:master Feb 23, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 23, 2023
@compiler-errors compiler-errors deleted the associated-type-bounds-in-bad-position branch August 11, 2023 19:55
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 18, 2024
…li-obk

Stabilize associated type bounds (RFC 2289)

This PR stabilizes associated type bounds, which were laid out in [RFC 2289]. This gives us a shorthand to express nested type bounds that would otherwise need to be expressed with nested `impl Trait` or broken into several `where` clauses.

### What are we stabilizing?

We're stabilizing the associated item bounds syntax, which allows us to put bounds in associated type position within other bounds, i.e. `T: Trait<Assoc: Bounds...>`. See [RFC 2289] for motivation.

In all position, the associated type bound syntax expands into a set of two (or more) bounds, and never anything else (see "How does this differ[...]" section for more info).

Associated type bounds are stabilized in four positions:
* **`where` clauses (and APIT)** - This is equivalent to breaking up the bound into two (or more) `where` clauses. For example, `where T: Trait<Assoc: Bound>` is equivalent to `where T: Trait, <T as Trait>::Assoc: Bound`.
* **Supertraits** - Similar to above, `trait CopyIterator: Iterator<Item: Copy> {}`. This is almost equivalent to breaking up the bound into two (or more) `where` clauses; however, the bound on the associated item is implied whenever the trait is used. See rust-lang#112573/rust-lang#112629.
* **Associated type item bounds** - This allows constraining the *nested* rigid projections that are associated with a trait's associated types. e.g. `trait Trait { type Assoc: Trait2<Assoc2: Copy>; }`.
* **opaque item bounds (RPIT, TAIT)** - This allows constraining associated types that are associated with the opaque without having to *name* the opaque. For example, `impl Iterator<Item: Copy>` defines an iterator whose item is `Copy` without having to actually name that item bound.

The latter three are not expressible in surface Rust (though for associated type item bounds, this will change in rust-lang#120752, which I don't believe should block this PR), so this does represent a slight expansion of what can be expressed in trait bounds.

### How does this differ from the RFC?

Compared to the RFC, the current implementation *always* desugars associated type bounds to sets of `ty::Clause`s internally. Specifically, it does *not* introduce a position-dependent desugaring as laid out in [RFC 2289], and in particular:
* It does *not* desugar to anonymous associated items in associated type item bounds.
* It does *not* desugar to nested RPITs in RPIT bounds, nor nested TAITs in TAIT bounds.

This position-dependent desugaring laid out in the RFC existed simply to side-step limitations of the trait solver, which have mostly been fixed in rust-lang#120584. The desugaring laid out in the RFC also added unnecessary complication to the design of the feature, and introduces its own limitations to, for example:
* Conditionally lowering to nested `impl Trait` in certain positions such as RPIT and TAIT means that we inherit the limitations of RPIT/TAIT, namely lack of support for higher-ranked opaque inference. See this code example: rust-lang#120752 (comment).
* Introducing anonymous associated types makes traits no longer object safe, since anonymous associated types are not nameable, and all associated types must be named in `dyn` types.

This last point motivates why this PR is *not* stabilizing support for associated type bounds in `dyn` types, e.g, `dyn Assoc<Item: Bound>`. Why? Because `dyn` types need to have *concrete* types for all associated items, this would necessitate a distinct lowering for associated type bounds, which seems both complicated and unnecessary compared to just requiring the user to write `impl Trait` themselves. See rust-lang#120719.

### Implementation history:

Limited to the significant behavioral changes and fixes and relevant PRs, ping me if I left something out--
* rust-lang#57428
* rust-lang#108063
* rust-lang#110512
* rust-lang#112629
* rust-lang#120719
* rust-lang#120584

Closes rust-lang#52662

[RFC 2289]: https://rust-lang.github.io/rfcs/2289-associated-type-bounds.html
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 19, 2024
…-obk

Stabilize associated type bounds (RFC 2289)

This PR stabilizes associated type bounds, which were laid out in [RFC 2289]. This gives us a shorthand to express nested type bounds that would otherwise need to be expressed with nested `impl Trait` or broken into several `where` clauses.

### What are we stabilizing?

We're stabilizing the associated item bounds syntax, which allows us to put bounds in associated type position within other bounds, i.e. `T: Trait<Assoc: Bounds...>`. See [RFC 2289] for motivation.

In all position, the associated type bound syntax expands into a set of two (or more) bounds, and never anything else (see "How does this differ[...]" section for more info).

Associated type bounds are stabilized in four positions:
* **`where` clauses (and APIT)** - This is equivalent to breaking up the bound into two (or more) `where` clauses. For example, `where T: Trait<Assoc: Bound>` is equivalent to `where T: Trait, <T as Trait>::Assoc: Bound`.
* **Supertraits** - Similar to above, `trait CopyIterator: Iterator<Item: Copy> {}`. This is almost equivalent to breaking up the bound into two (or more) `where` clauses; however, the bound on the associated item is implied whenever the trait is used. See rust-lang#112573/rust-lang#112629.
* **Associated type item bounds** - This allows constraining the *nested* rigid projections that are associated with a trait's associated types. e.g. `trait Trait { type Assoc: Trait2<Assoc2: Copy>; }`.
* **opaque item bounds (RPIT, TAIT)** - This allows constraining associated types that are associated with the opaque without having to *name* the opaque. For example, `impl Iterator<Item: Copy>` defines an iterator whose item is `Copy` without having to actually name that item bound.

The latter three are not expressible in surface Rust (though for associated type item bounds, this will change in rust-lang#120752, which I don't believe should block this PR), so this does represent a slight expansion of what can be expressed in trait bounds.

### How does this differ from the RFC?

Compared to the RFC, the current implementation *always* desugars associated type bounds to sets of `ty::Clause`s internally. Specifically, it does *not* introduce a position-dependent desugaring as laid out in [RFC 2289], and in particular:
* It does *not* desugar to anonymous associated items in associated type item bounds.
* It does *not* desugar to nested RPITs in RPIT bounds, nor nested TAITs in TAIT bounds.

This position-dependent desugaring laid out in the RFC existed simply to side-step limitations of the trait solver, which have mostly been fixed in rust-lang#120584. The desugaring laid out in the RFC also added unnecessary complication to the design of the feature, and introduces its own limitations to, for example:
* Conditionally lowering to nested `impl Trait` in certain positions such as RPIT and TAIT means that we inherit the limitations of RPIT/TAIT, namely lack of support for higher-ranked opaque inference. See this code example: rust-lang#120752 (comment).
* Introducing anonymous associated types makes traits no longer object safe, since anonymous associated types are not nameable, and all associated types must be named in `dyn` types.

This last point motivates why this PR is *not* stabilizing support for associated type bounds in `dyn` types, e.g, `dyn Assoc<Item: Bound>`. Why? Because `dyn` types need to have *concrete* types for all associated items, this would necessitate a distinct lowering for associated type bounds, which seems both complicated and unnecessary compared to just requiring the user to write `impl Trait` themselves. See rust-lang#120719.

### Implementation history:

Limited to the significant behavioral changes and fixes and relevant PRs, ping me if I left something out--
* rust-lang#57428
* rust-lang#108063
* rust-lang#110512
* rust-lang#112629
* rust-lang#120719
* rust-lang#120584

Closes rust-lang#52662

[RFC 2289]: https://rust-lang.github.io/rfcs/2289-associated-type-bounds.html
GuillaumeGomez pushed a commit to GuillaumeGomez/rust that referenced this pull request Jul 10, 2024
…-obk

Stabilize associated type bounds (RFC 2289)

This PR stabilizes associated type bounds, which were laid out in [RFC 2289]. This gives us a shorthand to express nested type bounds that would otherwise need to be expressed with nested `impl Trait` or broken into several `where` clauses.

### What are we stabilizing?

We're stabilizing the associated item bounds syntax, which allows us to put bounds in associated type position within other bounds, i.e. `T: Trait<Assoc: Bounds...>`. See [RFC 2289] for motivation.

In all position, the associated type bound syntax expands into a set of two (or more) bounds, and never anything else (see "How does this differ[...]" section for more info).

Associated type bounds are stabilized in four positions:
* **`where` clauses (and APIT)** - This is equivalent to breaking up the bound into two (or more) `where` clauses. For example, `where T: Trait<Assoc: Bound>` is equivalent to `where T: Trait, <T as Trait>::Assoc: Bound`.
* **Supertraits** - Similar to above, `trait CopyIterator: Iterator<Item: Copy> {}`. This is almost equivalent to breaking up the bound into two (or more) `where` clauses; however, the bound on the associated item is implied whenever the trait is used. See rust-lang#112573/rust-lang#112629.
* **Associated type item bounds** - This allows constraining the *nested* rigid projections that are associated with a trait's associated types. e.g. `trait Trait { type Assoc: Trait2<Assoc2: Copy>; }`.
* **opaque item bounds (RPIT, TAIT)** - This allows constraining associated types that are associated with the opaque without having to *name* the opaque. For example, `impl Iterator<Item: Copy>` defines an iterator whose item is `Copy` without having to actually name that item bound.

The latter three are not expressible in surface Rust (though for associated type item bounds, this will change in rust-lang#120752, which I don't believe should block this PR), so this does represent a slight expansion of what can be expressed in trait bounds.

### How does this differ from the RFC?

Compared to the RFC, the current implementation *always* desugars associated type bounds to sets of `ty::Clause`s internally. Specifically, it does *not* introduce a position-dependent desugaring as laid out in [RFC 2289], and in particular:
* It does *not* desugar to anonymous associated items in associated type item bounds.
* It does *not* desugar to nested RPITs in RPIT bounds, nor nested TAITs in TAIT bounds.

This position-dependent desugaring laid out in the RFC existed simply to side-step limitations of the trait solver, which have mostly been fixed in rust-lang#120584. The desugaring laid out in the RFC also added unnecessary complication to the design of the feature, and introduces its own limitations to, for example:
* Conditionally lowering to nested `impl Trait` in certain positions such as RPIT and TAIT means that we inherit the limitations of RPIT/TAIT, namely lack of support for higher-ranked opaque inference. See this code example: rust-lang#120752 (comment).
* Introducing anonymous associated types makes traits no longer object safe, since anonymous associated types are not nameable, and all associated types must be named in `dyn` types.

This last point motivates why this PR is *not* stabilizing support for associated type bounds in `dyn` types, e.g, `dyn Assoc<Item: Bound>`. Why? Because `dyn` types need to have *concrete* types for all associated items, this would necessitate a distinct lowering for associated type bounds, which seems both complicated and unnecessary compared to just requiring the user to write `impl Trait` themselves. See rust-lang#120719.

### Implementation history:

Limited to the significant behavioral changes and fixes and relevant PRs, ping me if I left something out--
* rust-lang#57428
* rust-lang#108063
* rust-lang#110512
* rust-lang#112629
* rust-lang#120719
* rust-lang#120584

Closes rust-lang#52662

[RFC 2289]: https://rust-lang.github.io/rfcs/2289-associated-type-bounds.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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.

Associated type bounds ICE
5 participants