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

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

Open
nikomatsakis opened this issue Feb 22, 2018 · 5 comments
Labels
A-specialization Area: Trait impl specialization A-traits Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. F-specialization `#![feature(specialization)]` requires-nightly This issue requires a nightly compiler in some way. S-types-deferred Status: Identified as a valid potential future enhancement that is not currently being worked on 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

@nikomatsakis
Copy link
Contributor

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 nikomatsakis added A-traits Area: Trait system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-specialization Area: Trait impl specialization labels Feb 22, 2018
@nikomatsakis
Copy link
Contributor Author

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 nikomatsakis added the WG-traits Working group: Traits, https://internals.rust-lang.org/t/announcing-traits-working-group/6804 label Feb 25, 2018
@nikomatsakis nikomatsakis changed the title specialization permits empty impls when parent has no default items 🛠️ specialization permits empty impls when parent has no default items Feb 25, 2018
@nikomatsakis
Copy link
Contributor Author

The main question is, is this even a bug?

@varkor
Copy link
Member

varkor commented Feb 25, 2018

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
Copy link
Contributor Author

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. #41756. Also, I wonder if this aspect (the affects on type inference) is a duplicate of #46363...

@jkordish jkordish added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Apr 18, 2018
@Diggsey
Copy link
Contributor

Diggsey commented Feb 27, 2019

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

@jonas-schievink jonas-schievink added F-specialization `#![feature(specialization)]` requires-nightly This issue requires a nightly compiler in some way. labels Sep 15, 2019
@jackh726 jackh726 added S-types-deferred Status: Identified as a valid potential future enhancement that is not currently being worked on T-types Relevant to the types team, which will review and decide on the PR/issue. and removed WG-traits Working group: Traits, https://internals.rust-lang.org/t/announcing-traits-working-group/6804 labels Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-specialization Area: Trait impl specialization A-traits Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. F-specialization `#![feature(specialization)]` requires-nightly This issue requires a nightly compiler in some way. S-types-deferred Status: Identified as a valid potential future enhancement that is not currently being worked on 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