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 error when using an associated type as a type parameter in the super-trait listing #40533

Open
jonysy opened this issue Mar 15, 2017 · 6 comments
Labels
A-traits Area: Trait system A-typesystem Area: The type system C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@jonysy
Copy link

jonysy commented Mar 15, 2017

I tried this code:

trait Super<T> {}

trait Foo: Super<<Self as Foo>::Bar> { type Bar;  }

type BoxFoo = Box<Foo<Bar = u8>>;

The code should compile since Self::Bar is the type parameter, not Self.

Instead, it will fail with the following message:

the trait Foo cannot be made into an object
the trait cannot use Self as a type parameter in the supertrait listing

FYI the discussion that sparked this issue:

So this has to do with object safety -- any trait that references Self is not object safe because the true Self type is erased. However, the error doesn't seem appropriate in this case. I think this warrants a bug report at least to fix the error message, if not to relax the object safety check (because there could be something I don't see preventing it still... can't say I fully understand object safety).

Meta

rustc --version --verbose: rustc 1.17.0-nightly (b1e31766d 2017-03-03)

@Mark-Simulacrum Mark-Simulacrum added A-diagnostics Area: Messages for errors, warnings, and lints A-typesystem Area: The type system T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jun 13, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@estebank
Copy link
Contributor

Current output:

error[E0038]: the trait `Foo` cannot be made into an object
 --> src/lib.rs:5:19
  |
5 | type BoxFoo = Box<Foo<Bar = u8>>;
  |                   ^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object
  |
  = note: the trait cannot use `Self` as a type parameter in the supertraits or where-clauses

It should at least point at trait Foo's supertraits.

@estebank estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 11, 2019
@estebank
Copy link
Contributor

estebank commented Feb 2, 2020

Output in #68377:

error[E0038]: the trait `Foo` cannot be made into an object
 --> file.rs:7:19
  |
3 | trait Foo: Super<<Self as Foo>::Bar> {
  |       ---  ------------------------- ...because it cannot use `Self` as a type parameter in the supertraits or `where`-clauses
  |       |
  |       this trait cannot be made into an object...
...
7 | type BoxFoo = Box<dyn Foo<Bar = u8>>;
  |                   ^^^^^^^^^^^^^^^^^ the trait `Foo` cannot be made into an object

bors added a commit that referenced this issue Feb 4, 2020
Tweak obligation error output

- Point at arguments or output when fn obligations come from them, or ident when they don't
- Point at `Sized` bound (fix #47990)
- When object unsafe trait uses itself in associated item suggest using `Self` (fix #66424, fix #33375, partially address #38376, cc #61525)
-  Point at reason in object unsafe trait with `Self` in supertraits or `where`-clause (cc #40533, cc #68377)
- On implicit type parameter `Sized` obligations, suggest `?Sized` (fix #57744, fix #46683)
@estebank
Copy link
Contributor

estebank commented Dec 13, 2022

I believe that we should change the implementation here

let self_ty = tcx.types.self_param;
let has_self_ty = |arg: &GenericArg<'tcx>| arg.walk().any(|arg| arg == self_ty.into());
match predicate.kind().skip_binder() {
ty::PredicateKind::Clause(ty::Clause::Trait(ref data)) => {
// In the case of a trait predicate, we can skip the "self" type.
if data.trait_ref.substs[1..].iter().any(has_self_ty) { Some(sp) } else { None }
}

to account for data=TraitPredicate(<Self as Super<<Self as Foo>::Bar>>, polarity:Positive and explicitly allow it, by ignoring when Self is only referenced to access one of its currently defined associated types.

Further changes would also be needed here

// Like for trait refs, verify that `dummy_self` did not leak inside default type
// parameters.
let references_self = b.projection_ty.substs.iter().skip(1).any(|arg| {
if arg.walk().any(|arg| arg == dummy_self.into()) {
return true;
}
false
});
if references_self {
tcx.sess
.delay_span_bug(span, "trait object projection bounds reference `Self`");
let substs: Vec<_> = b
.projection_ty
.substs
.iter()
.map(|arg| {
if arg.walk().any(|arg| arg == dummy_self.into()) {
return tcx.ty_error().into();
}
arg
})
.collect();
b.projection_ty.substs = tcx.intern_substs(&substs[..]);
}

@rust-lang/lang, would this require team sign-off, or is the current behavior "accidental"/"incidental" and a change of behavior requires no "new rationale"?

I'm also intrigued on whether this test should have been rejected with a more targeted error, instead of E0038: https://github.com/rust-lang/rust/blob/eb4580a570069175e1290b294d91042a09f9fde3/src/test/ui/issues/issue-26056.rs

@estebank estebank removed the A-diagnostics Area: Messages for errors, warnings, and lints label Dec 13, 2022
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Dec 13, 2022

@estebank

Self can appear in projections, but only as part of an "identity" trait-ref. i.e., if you hvae trait Foo<A..Z>, then the trait ref has to be <Self as Foo<A..Z>>::Bar. This ensures that we can extract the associated type from the self type (which will be something like dyn Foo<A..Z, Bar=X>.

I think such a PR does not require an FCP, but it's prob a good idea to nominate for @rust-lang/types approval or review. This feels to me like a 'nitty gritty' detail. I think in general @rust-lang/types is a good choice for that, and types team can escalate to lang if it seems like it requires a policy decision.

@estebank estebank added T-types Relevant to the types team, which will review and decide on the PR/issue. A-traits Area: Trait system and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. D-confusing Diagnostics: Confusing error or lint that should be reworked. labels Dec 13, 2022
@ZhennanWu
Copy link

This is a disparity between trait generic parameter and associated type. I very much doubt such disparity is intended in this scenario. Though not particularly damaging, it adds restrictions during high-level type designs.

trait Super<T> {}

trait Foo<Bar>: Super<Bar> {} // Using a generic parameter does not break object safety

type BoxFoo = Box<Foo<u8>>;

@lcnr
Copy link
Contributor

lcnr commented Jul 6, 2023

I think such a PR does not require an FCP, but it's prob a good idea to nominate for @rust-lang/types approval or review. This feels to me like a 'nitty gritty' detail. I think in general @rust-lang/types is a good choice for that, and types team can escalate to lang if it seems like it requires a policy decision.

I quite strongly believe that a fix for this issue should require a types FCP. Changes to the stable behavior of the type system, especially if the changes are hanbdle some nitty gritty detail should go through a types FCP, see https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/FCPs.20for.20type.20system.20hacks where we've previously discussed this question on zulip. Most unsound issues and weird backcompat hurdles tend to be caused by one (or the interaction of multiple) such nitty gritty detail(s). Even if the FCP only manages to force t-types members to look at the PR in more detail, this is still very clearly worth it to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system A-typesystem Area: The type system C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants