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

Adding a specialized impl can break inference. #46363

Open
leodasvacas opened this Issue Nov 29, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@leodasvacas
Copy link
Contributor

leodasvacas commented Nov 29, 2017

Relevant to #31844. This is how it happens:

#![feature(specialization)]

trait B { fn b(&self) -> Self; }

// Impl 1
impl<T> B for Option<T> where T: Default
{
    default fn b(&self) -> Option<T> { Some(T::default()) }
}
// Removing one of the two concrete impls makes inference succeed.
// Impl 2
impl B for Option<String>
{
    fn b(&self) -> Self { Some("special".into()) }
}
// Impl 3
impl B for Option<i32>
{
    fn b(&self) -> Self { Some(0) }
}

fn main() { 
    // Cannot infer `T` in `Option<T>`.
   None.b();
}

This issue does not originate from specialization, since if we removed Impl 1 the same problem would occur. But with specialization if those impls were added in order, the story would be a bit confusing:

  1. With Impl 1, inference fails.
  2. Added Impl 2, yay inference succeeds.
  3. Added Impl 3, inference is back to failing.

The only fix would be to make inference fail in step 2. Even if it's not something we want fix it still seems worth noting somewhere that we are ok with specializaton breaking inference.

@joshlf

This comment has been minimized.

Copy link
Contributor

joshlf commented Jun 10, 2018

Is this the same issue that's causing this error? #51464 (comment)

There used to be a bunch of specific impls for SliceIndex for various types. That PR adds a default impl (so the old specific impls are now specializations of the default impl). The error in question is a test case that used to pass that now fails inference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.