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

object-safety removed with type AssocType: PartialEq #122798

Closed
RustyYato opened this issue Mar 20, 2024 · 3 comments
Closed

object-safety removed with type AssocType: PartialEq #122798

RustyYato opened this issue Mar 20, 2024 · 3 comments
Assignees
Labels
A-type-system Area: Type system C-bug Category: This is a bug. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RustyYato
Copy link
Contributor

I tried this code:

pub trait Foo {
    type X: PartialEq;
}

pub trait Bar {
    type Y: Eq;
}

// this doesn't work for some reason!!!
// fn get_foo<T: Foo>(t: &T) -> &dyn Foo<X = T::X> {
//     t
// }

fn get_bar<T: Bar>(t: &T) -> &dyn Bar<Y = T::Y> {
    t
}

If I uncomment out get_foo, I get an error, but get_bar doesn't error!
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f20eef39a18a6fdc11cbeea9778ee294

To me this makes no sense since Bar is more strict than Foo, so either: both should not compile or Foo should compile (Bar may not since it has more requirements on it's associated type).

What's more fun is that this used to compile way back in v1.15.1 according to godbolt 😄 https://godbolt.org/z/TvPsjPq5o
But no version after that

Meta

playground is on v1.76.0 when I ran this

rustc --version --verbose:

rustc 1.79.0-nightly (3c85e5624 2024-03-18)
binary: rustc
commit-hash: 3c85e56249b0b1942339a6a989a971bf6f1c9e0f
commit-date: 2024-03-18
host: x86_64-unknown-linux-gnu
release: 1.79.0-nightly
LLVM version: 18.1.2
Compile Error

error[E0038]: the trait `Foo` cannot be made into an object
  --> src/lib.rs:10:31
   |
10 | fn get_foo<T: Foo>(t: &T) -> &dyn Foo<X = T::X> {
   |                               ^^^^^^^^^^^^^^^^^ `Foo` cannot be made into an object
   |
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
  --> src/lib.rs:2:13
   |
1  | pub trait Foo {
   |           --- this trait cannot be made into an object...
2  |     type X: PartialEq;
   |             ^^^^^^^^^ ...because it uses `Self` as a type parameter

For more information about this error, try `rustc --explain E0038`.
error: could not compile `playground` (lib) due to 1 previous error

@RustyYato RustyYato added the C-bug Category: This is a bug. label Mar 20, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 20, 2024
@jieyouxu jieyouxu added A-type-system Area: Type system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 20, 2024
@compiler-errors compiler-errors self-assigned this Mar 21, 2024
Noratrieb added a commit to Noratrieb/rust that referenced this issue Jun 4, 2024
…ference-self, r=BoxyUwU

Item bounds can reference self projections and still be object safe

### Background

Currently, we have some interesting rules about where `Self` is allowed to be mentioned in objects. Specifically, we allow mentioning `Self` behind associated types (e.g. `fn foo(&self) -> Self::Assoc`) only if that `Self` type comes from the trait we're defining or its supertraits:

```
trait Foo {
  fn good() -> Self::Assoc; // GOOD :)

  fn bad() -> <Self as OtherTrait>::Assoc; // BAD!
}
```

And more specifically, these `Self::Assoc` projections are *only* allowed to show up in:
  * (A1) Method signatures
  * (A2) Where clauses on traits, GATs and methods

But `Self::Assoc` projections are **not** allowed to show up in:
  * (B1) Supertrait bounds (specifically: all *super-predicates*, which includes the projections that come from elaboration, and not just the traits themselves).
  * (B2) Item bounds of associated types

The reason for (B1) is interesting: specifically, it arises from the fact that we currently eagerly elaborate all projection predicates into the object, so if we had the following code:

```
trait Sub<Assoc = Self::SuperAssoc> {}
trait Super {
    type SuperAssoc;
}
```

Then given `dyn Sub<SuperAssoc = i32>` we would need to have a type that is substituted into itself an infinite number of times[^1], like `dyn Sub<SuperAssoc = i32, Assoc = <dyn Sub<SuperAssoc = i32, Assoc = <dyn Sub<SuperAssoc = i32, Assoc = <... as Super>::SuperAssoc> as Super>::SuperAssoc> as Super>::SuperAssoc>`, i.e. the fixed-point of: `type T = dyn Sub<SuperAssoc = i32, Assoc = <T as Super>::SuperAssoc>`.

Similarly for (B2), we restrict mentioning `Self::Assoc` in associated type item bounds, which is the cause for rust-lang#122798. However, there is **no reason** for us to do so, since item bounds never show up structurally in the `dyn Trait` object type.

#### What?

This PR relaxes the check for item bounds so that `Self` may be mentioned behind associated types in the same cases that they currently work for method signatures (A1) and where clauses (A2).

#### Why?

Fixes rust-lang#122798. Removes a subtle and confusing inconsistency for the code mentioned in that issue.

This is sound because we only assemble alias bounds for rigid projections, and all projections coming from an object self type are not rigid, since all associated types should be specified by the type.

This is also desirable because we can do this via supertraits already. In rust-lang#122789, it is noted that an item bound of `Eq` already works, just not `PartialEq` because of the default item bound. This is weird and should be fixed.

#### Future work

We could make the check for `Self` in super-predicates more sophisticated as well, only erroring if `Self` shows up in a projection super-predicate.

[^1]: This could be fixed by some sort of structural replacement or eager normalization, but I don't think it's necessary currently.
Noratrieb added a commit to Noratrieb/rust that referenced this issue Jun 4, 2024
…ference-self, r=BoxyUwU

Item bounds can reference self projections and still be object safe

### Background

Currently, we have some interesting rules about where `Self` is allowed to be mentioned in objects. Specifically, we allow mentioning `Self` behind associated types (e.g. `fn foo(&self) -> Self::Assoc`) only if that `Self` type comes from the trait we're defining or its supertraits:

```
trait Foo {
  fn good() -> Self::Assoc; // GOOD :)

  fn bad() -> <Self as OtherTrait>::Assoc; // BAD!
}
```

And more specifically, these `Self::Assoc` projections are *only* allowed to show up in:
  * (A1) Method signatures
  * (A2) Where clauses on traits, GATs and methods

But `Self::Assoc` projections are **not** allowed to show up in:
  * (B1) Supertrait bounds (specifically: all *super-predicates*, which includes the projections that come from elaboration, and not just the traits themselves).
  * (B2) Item bounds of associated types

The reason for (B1) is interesting: specifically, it arises from the fact that we currently eagerly elaborate all projection predicates into the object, so if we had the following code:

```
trait Sub<Assoc = Self::SuperAssoc> {}
trait Super {
    type SuperAssoc;
}
```

Then given `dyn Sub<SuperAssoc = i32>` we would need to have a type that is substituted into itself an infinite number of times[^1], like `dyn Sub<SuperAssoc = i32, Assoc = <dyn Sub<SuperAssoc = i32, Assoc = <dyn Sub<SuperAssoc = i32, Assoc = <... as Super>::SuperAssoc> as Super>::SuperAssoc> as Super>::SuperAssoc>`, i.e. the fixed-point of: `type T = dyn Sub<SuperAssoc = i32, Assoc = <T as Super>::SuperAssoc>`.

Similarly for (B2), we restrict mentioning `Self::Assoc` in associated type item bounds, which is the cause for rust-lang#122798. However, there is **no reason** for us to do so, since item bounds never show up structurally in the `dyn Trait` object type.

#### What?

This PR relaxes the check for item bounds so that `Self` may be mentioned behind associated types in the same cases that they currently work for method signatures (A1) and where clauses (A2).

#### Why?

Fixes rust-lang#122798. Removes a subtle and confusing inconsistency for the code mentioned in that issue.

This is sound because we only assemble alias bounds for rigid projections, and all projections coming from an object self type are not rigid, since all associated types should be specified by the type.

This is also desirable because we can do this via supertraits already. In rust-lang#122789, it is noted that an item bound of `Eq` already works, just not `PartialEq` because of the default item bound. This is weird and should be fixed.

#### Future work

We could make the check for `Self` in super-predicates more sophisticated as well, only erroring if `Self` shows up in a projection super-predicate.

[^1]: This could be fixed by some sort of structural replacement or eager normalization, but I don't think it's necessary currently.
@SteveLauC
Copy link
Contributor

SteveLauC commented Oct 29, 2024

Also hit this, in my case, if the associated type has a PartialOrd bound, then the trait is not object-safe, changing the trait bound to Ord makes things work.

@compiler-errors
Copy link
Member

Oh, this is fixed as of 1.82 -- if you still have a problem, please file a new issue.

@SteveLauC
Copy link
Contributor

Oh, this is fixed as of 1.82

Indeed, using the latest nightly toolchain, the error disappears! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-bug Category: This is a bug. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants