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

Implicit resolution inconsistency #5354

Closed
scabug opened this Issue Jan 3, 2012 · 5 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

commented Jan 3, 2012

The following two programs should both give ambiguity errors, but only the first one does:

package object foo {
  implicit val x: Bippy = new Bippy("x")
}
package foo {
  class Bippy(override val toString: String) { }
  class Dingus {
    implicit val z: Bippy = new Bippy("z")
    def f1 = implicitly[Bippy]
  }
}

This one compiles without error, but shouldn't:

package object foo {
  implicit val x: Bippy = new Bippy("x")
}
package foo {
  class Bippy(override val toString: String){ }
  class Dingus {
    def f1 = {
      implicit val z: Bippy = new Bippy("z")
      implicitly[Bippy]
    }
  }
@scabug

This comment has been minimized.

Copy link
Author

commented Jan 3, 2012

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 3, 2012

@odersky said:
I think I have isolated the reason. Overloading resolution prefers values x over parameterless method accessors x. In the first case, we have two implicits as fields, and both are accessed with parameterless method accessors. So ambiguity results. In the second case, the local variable is treated as more specific in overloading resolution. This is not justified by the spec, which treats fields and parameterless methods the same. But changing it might break existing code, so we have to tread carefully here.

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 4, 2012

@odersky said:
Turns out my previous reasoning was completely incorrect. The compiler does indeed treat values and parameterless methods the same, just as the spec demands. The reason why the second example slipped through is considerably more devious: When checking the Foo.x' implicit, a CyclicReference error occurs which causes the alternative to be discarded. Why a CylicReference? Because the inferencer tries to decide whether the owner of zis a subclass of the owner odx. To do this, it computed the info of the owner of z1, which is not complete because no result type for f1` was given. Hence a CyclicReference error.

The fix is twofold: (1) We make isNonBottomSubClass smarter so that it always returns false if the symbol in question is not a type; hence the info need not be computed. (2) It's dubious to swallow CyclicReference errors anywhere, but I deemed it too risky to propagate them. But at least the CyclicReference is now logged if -Ylog-implicit is true. This hopefully spares future maintainers the same detective work I had to go through when digging this out.

Leaving for Paul to close once pull request is accepted.

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 4, 2012

@paulp said:
Merged in 6975b4888d.

@scabug scabug closed this Jan 4, 2012

@scabug

This comment has been minimized.

Copy link
Author

commented May 27, 2012

@retronym said:
This also fixed a similar problem in a different manner:

http://stackoverflow.com/questions/10763511/implicit-resolution-in-scala-2-10-x-whats-going-on

2.9.2 considered <local Test> as a subclass of <none>.

@scabug scabug added this to the 2.10.0 milestone Apr 7, 2017

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.