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

Regression in associated type checking for default impls #73818

Closed
jsmith628 opened this issue Jun 27, 2020 · 5 comments · Fixed by #74670
Closed

Regression in associated type checking for default impls #73818

jsmith628 opened this issue Jun 27, 2020 · 5 comments · Fixed by #74670
Labels
A-associated-items Area: Associated items (types, constants & functions) C-bug Category: This is a bug. F-specialization `#![feature(specialization)]` requires-nightly This issue requires a nightly compiler in some way.

Comments

@jsmith628
Copy link

So I had a trait system in my (nightly) code for a while now that utilized specialization, but it no longer seems to be working after a recent update:

#![feature(specialization)]

trait Trait1 { type Selection: PartialEq; }

trait Trait2: PartialEq<Self> {}
impl<T:Trait2> Trait1 for T { default type Selection = T; }
warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes
 --> src/main.rs:1:12
  |
1 | #![feature(specialization)]
  |            ^^^^^^^^^^^^^^
  |
  = note: `#[warn(incomplete_features)]` on by default
  = note: see issue #31844 <https://github.com/rust-lang/rust/issues/31844> for more information

error[E0277]: can't compare `T` with `<T as Trait1>::Selection`
 --> src/main.rs:6:31
  |
3 | trait Trait1 { type Selection: PartialEq; }
  |                -------------------------- required by `Trait1::Selection`
...
6 | impl<T:Trait2> Trait1 for T { default type Selection = T; }
  |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `T == <T as Trait1>::Selection`
  |
  = help: the trait `std::cmp::PartialEq<<T as Trait1>::Selection>` is not implemented for `T`
help: consider further restricting this bound
  |
6 | impl<T:Trait2 + std::cmp::PartialEq<<T as Trait1>::Selection>> Trait1 for T { default type Selection = T; }
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.

Now, I haven't been following the development of the specialization feature too closely, so it is possible that this is somehow actually intended and I am missing something. However, given that this previously compiled and ran just fine a couple weeks ago and given that the specialization incomplete warning was only added recently, I am inclined to think that it is probably a regression of some sort.

Meta things

compiler version:

$rustc --version --verbose
rustc 1.46.0-nightly (7750c3d46 2020-06-26)
binary: rustc
commit-hash: 7750c3d46bc19784adb1ee6e37a5ec7e4cd7e772
commit-date: 2020-06-26
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0
@jsmith628 jsmith628 added the C-bug Category: This is a bug. label Jun 27, 2020
@jonas-schievink jonas-schievink added F-specialization `#![feature(specialization)]` requires-nightly This issue requires a nightly compiler in some way. A-associated-items Area: Associated items (types, constants & functions) E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Jun 27, 2020
@tmandry
Copy link
Member

tmandry commented Jul 9, 2020

Another MVCE (playground):

#![feature(specialization)]

#[derive(PartialEq)]
enum Never {}

trait Foo {
    type Assoc: PartialEq;
}

impl<T> Foo for T {
    default type Assoc = Never;
}

Regressed in 7058471, looks like #72788

@tmandry tmandry removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Jul 9, 2020
@tmandry
Copy link
Member

tmandry commented Jul 9, 2020

cc @matthewjasper, as far as I can tell this wasn't an intended outcome of the change?

@matthewjasper
Copy link
Contributor

It wasn't really, it was just slightly more complex to have this still work after that PR.

@tmandry
Copy link
Member

tmandry commented Jul 9, 2020

It wasn't really, it was just slightly more complex to have this still work after that PR.

@matthewjasper Would you mind describing at a high level what needs to be done?

@tmandry
Copy link
Member

tmandry commented Jul 22, 2020

It looks like the problem stems from the bound predicate not being fully normalized when checking the defaulted associated type. From my reproducer above:

compare_projection_bounds: normalized predicate =
    Binder(TraitPredicate(<Never as std::cmp::PartialEq<<T as Foo>::Assoc>>))

which should instead be <Never as std::cmp::PartialEq<Never>>.

I'm not certain yet how this ought to be implemented. Perhaps a pass after normalization that looks for occurrences of the defaulted type (<T as Foo>::Assoc in this case) and replaces it with the defaulted type. Seems simple enough, in theory :) But maybe there are cases I'm not thinking of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) C-bug Category: This is a bug. F-specialization `#![feature(specialization)]` requires-nightly This issue requires a nightly compiler in some way.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants