Skip to content

Conversation

@odersky
Copy link
Contributor

@odersky odersky commented Sep 18, 2015

Change inference scheme to look at how type variables are constrained
instead of just taking into account the variance in which they occur in some
type. Review by @smarter.

Change algorithm that determines whether type variables are
minimized or maximized. We used to look only at the variance
type variable in the containing type. We now also look with
higher precedence at the direction from which the type variable
was constrained. This is closer to what scalac does.
by adding the following rule:

Before typing an implicit parameter list of a method m, instantiate all type parameters of m that occur in the type of some preceding value parameter of m.
(scalac and dotty both produce an error here)
We now also consider type variables in a selection prefix of the
application. The test case was augmented to include a snippet which
only succeeds under the generalization.
@odersky
Copy link
Contributor Author

odersky commented Oct 6, 2015

Can we get this reviewed please? it's been lingering for a long time.

@smarter
Copy link
Member

smarter commented Oct 6, 2015

Yes, I'm on it

@smarter
Copy link
Member

smarter commented Oct 7, 2015

Overall I think this PR is a good improvement on the current situation but there are still things which don't work correctly, for example:

object Test {
  def one[T](x: T)(implicit ev: T): Nothing = ???

  def test = {
    implicit val ii: Int = 42

    one(10)
    // error: ambiguous implicits: both getter StringCanBuildFrom in object Predef$
    // and getter NothingClassTag in object DottyPredef$ match type Any of
    // parameter ev of method one in object Test$
  }
}

I'm working on fixing this(it should only require us to be less eager to instantiate in interpolateUndetVars) and other issues. I'm not sure if the new design of "interpolate in the direction that was constrained" is the correct thing to do, but we can always revert that in the future if it ends up being a problem.

@odersky
Copy link
Contributor Author

odersky commented Oct 7, 2015

OK, should we get this in then and do further improvements in a separate PR?

@smarter
Copy link
Member

smarter commented Oct 7, 2015

Yes, sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants