rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.76k stars 12.65k forks source link

🛠️ specialization permits empty impls when parent has no default items #48444

Open nikomatsakis opened 6 years ago

nikomatsakis commented 6 years ago

It is potentially somewhat surprising that the following impls are accepted:

#![feature(specialization)]

trait Foo {
    fn bar();
}

impl<T> Foo for T {
    fn bar() { } // no default
}

impl Foo for i32 {
    // OK so long as we define no items:
    // fn bar() { }
}

fn main() {}

The original example was unearthed by @eddyb and @varkor as part of a PR.

To be clear, I don't see this as a soundness problem per se. Just a question of what we think it ought to do.

cc @aturon

nikomatsakis commented 6 years ago

Also, this affects type inference in creepy ways. For example, this program will not build:

#![feature(specialization)]

struct X;
struct Y;
impl From<X> for Y {
    fn from(_: X) -> Y { Y }
}

// impl Into<Y> for X {}

fn main() {
    let x = X.into();
}

but if you uncomment the impl, it will.

nikomatsakis commented 6 years ago

The main question is, is this even a bug?

varkor commented 6 years ago

In the case of specialisation affecting type inference, I think this is very counter-intuitive and makes the behaviour hard to fathom without actually compiling the program, which I think we'd rather avoid...

nikomatsakis commented 6 years ago

In the case of specialisation affecting type inference, I think this is very counter-intuitive and makes the behaviour hard to fathom without actually compiling the program, which I think we'd rather avoid...

That's true, though I think that's a sort of separate problem really. We should probably extract that into its own issue. I suspect it is a by-product of some of the details of how trait selection works -- details I hope to change. See e.g. https://github.com/rust-lang/rust/issues/41756. Also, I wonder if this aspect (the affects on type inference) is a duplicate of https://github.com/rust-lang/rust/issues/46363...

Diggsey commented 5 years ago

This doesn't seem like a bug per se - perhaps the best thing to do is to emit a warning when an impl is redundant in this way? Something like Warning: "impl" block is redundant because i32 already implements Foo