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

Eagerly elaborate auto-trait supertraits into dyn Trait #119825

Closed

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Jan 10, 2024

When we have a trait like:

trait Foo: Send {}

Then the dyn Foo type is elaborated to include Send as part of its auto-traits list. That means it is the same type as dyn Foo + Send.

Why?

When a auto trait is implied as part of the supertraits of the trait object's principal, the user should never care about whether they explicitly write + AutoTrait or have it implied from the supertrait bounds. This PR removes this subtle distinction.

IMO, It never makes sense for dyn Foo to be a distinct type from dyn Foo + Send, since both implement Send, just via different code paths in the trait solver.


Alternative approach to #119338.

r? lcnr

@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 Jan 10, 2024
@rustbot rustbot added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Jan 10, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2024

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@compiler-errors
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 10, 2024
Eagerly elaborate auto-trait supertraits into `dyn Trait`

When we have a trait like:
```
trait Foo: Send {}
```

Then the `dyn Foo` type is elaborated to include `Send` as part of its auto-traits list. That means it is the *same* type as `dyn Foo + Send`.

## Why?

When a auto trait is implied as part of the supertraits of the trait object's principal, the user should *never* care about whether they explicitly write `+ AutoTrait` or have it implied from the supertrait bounds. This PR removes this subtle distinction.

IMO, It *never* makes sense for `dyn Foo` to be a distinct type from `dyn Foo + Send`, since both implement `Send`, just via different code paths in the trait solver.

---

Alternative approach to rust-lang#119338.

r? lcnr
@bors
Copy link
Contributor

bors commented Jan 10, 2024

⌛ Trying commit caeda85 with merge 6802c00...

@compiler-errors
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Jan 10, 2024

⌛ Trying commit dca631c with merge 925e66e...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 10, 2024
Eagerly elaborate auto-trait supertraits into `dyn Trait`

When we have a trait like:
```
trait Foo: Send {}
```

Then the `dyn Foo` type is elaborated to include `Send` as part of its auto-traits list. That means it is the *same* type as `dyn Foo + Send`.

## Why?

When a auto trait is implied as part of the supertraits of the trait object's principal, the user should *never* care about whether they explicitly write `+ AutoTrait` or have it implied from the supertrait bounds. This PR removes this subtle distinction.

IMO, It *never* makes sense for `dyn Foo` to be a distinct type from `dyn Foo + Send`, since both implement `Send`, just via different code paths in the trait solver.

---

Alternative approach to rust-lang#119338.

r? lcnr
@bors
Copy link
Contributor

bors commented Jan 10, 2024

☀️ Try build successful - checks-actions
Build commit: 925e66e (925e66ec5d7d73da31ddd13e53f21e4674feb4ba)

@compiler-errors
Copy link
Member Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-119825 created and queued.
🤖 Automatically detected try build 925e66e
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 10, 2024
@craterbot
Copy link
Collaborator

🚧 Experiment pr-119825 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-119825 is completed!
📊 2354 regressed and 6 fixed (406907 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jan 14, 2024
@lcnr
Copy link
Contributor

lcnr commented Jan 15, 2024

alright :/ seems like that's not possible, let go with the coercion 😅

@compiler-errors
Copy link
Member Author

yeet

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2024
…=lcnr

Consider principal trait ref's auto-trait super-traits in dyn upcasting

Given traits like:

```rust
trait Subtrait: Supertrait + Send {}
trait Supertrait {}
```

We should be able to upcast `dyn Subtrait` to `dyn Supertrait + Send`. This is not currently possible, because when upcasting, we look at the list of auto traits in the object type (`dyn Subtrait`, which has no auto traits in its bounds) and compare them to the target's auto traits (`dyn Supertrait + Send`, which has `Send` in its bound).

Since the target has auto traits that are not present in the source, the upcasting fails. This is overly restrictive, since `dyn Subtrait` will always implement `Send` via its built-in object impl. I propose to loosen this restriction here.

r? types

---

### ~~Aside: Fix this in astconv instead?~~

### edit: This causes too many failures. See rust-lang#119825 (comment)

We may also fix this by by automatically elaborating all auto-trait supertraits during `AstConv::conv_object_ty_poly_trait_ref`. That is, we can make it so that `dyn Subtrait` is elaborated into the same type of `dyn Subtrait + Send`.

I'm open to considering this solution instead, but it would break coherence in the following example:

```rust
trait Foo: Send {}

trait Bar {}
impl Bar for dyn Foo {}
impl Bar for dyn Foo + Send {}
//~^ This would begin to be an overlapping impl.
```
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
Consider principal trait ref's auto-trait super-traits in dyn upcasting

Given traits like:

```rust
trait Subtrait: Supertrait + Send {}
trait Supertrait {}
```

We should be able to upcast `dyn Subtrait` to `dyn Supertrait + Send`. This is not currently possible, because when upcasting, we look at the list of auto traits in the object type (`dyn Subtrait`, which has no auto traits in its bounds) and compare them to the target's auto traits (`dyn Supertrait + Send`, which has `Send` in its bound).

Since the target has auto traits that are not present in the source, the upcasting fails. This is overly restrictive, since `dyn Subtrait` will always implement `Send` via its built-in object impl. I propose to loosen this restriction here.

r? types

---

### ~~Aside: Fix this in astconv instead?~~

### edit: This causes too many failures. See rust-lang/rust#119825 (comment)

We may also fix this by by automatically elaborating all auto-trait supertraits during `AstConv::conv_object_ty_poly_trait_ref`. That is, we can make it so that `dyn Subtrait` is elaborated into the same type of `dyn Subtrait + Send`.

I'm open to considering this solution instead, but it would break coherence in the following example:

```rust
trait Foo: Send {}

trait Bar {}
impl Bar for dyn Foo {}
impl Bar for dyn Foo + Send {}
//~^ This would begin to be an overlapping impl.
```
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Consider principal trait ref's auto-trait super-traits in dyn upcasting

Given traits like:

```rust
trait Subtrait: Supertrait + Send {}
trait Supertrait {}
```

We should be able to upcast `dyn Subtrait` to `dyn Supertrait + Send`. This is not currently possible, because when upcasting, we look at the list of auto traits in the object type (`dyn Subtrait`, which has no auto traits in its bounds) and compare them to the target's auto traits (`dyn Supertrait + Send`, which has `Send` in its bound).

Since the target has auto traits that are not present in the source, the upcasting fails. This is overly restrictive, since `dyn Subtrait` will always implement `Send` via its built-in object impl. I propose to loosen this restriction here.

r? types

---

### ~~Aside: Fix this in astconv instead?~~

### edit: This causes too many failures. See rust-lang/rust#119825 (comment)

We may also fix this by by automatically elaborating all auto-trait supertraits during `AstConv::conv_object_ty_poly_trait_ref`. That is, we can make it so that `dyn Subtrait` is elaborated into the same type of `dyn Subtrait + Send`.

I'm open to considering this solution instead, but it would break coherence in the following example:

```rust
trait Foo: Send {}

trait Bar {}
impl Bar for dyn Foo {}
impl Bar for dyn Foo + Send {}
//~^ This would begin to be an overlapping impl.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot dyn upcast to target w/ auto traits implied by source's principal's supertraits
5 participants