Skip to content

Test Only: Revert interpolateTypeVars fix #6333

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Test Only: Revert interpolateTypeVars fix #6333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7fe7954
Optimize interpolateTypeVars
odersky Apr 17, 2019
35e6318
Another optimization
odersky Apr 17, 2019
e1a9c2f
Test Only: Revert interpolateTypeVars fix.
odersky Apr 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 53 additions & 50 deletions compiler/src/dotty/tools/dotc/typer/Inferencing.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,57 +410,60 @@ trait Inferencing { this: Typer =>
// anymore if they've been garbage-collected, so we can't use
// `state.ownedVars.size > locked.size` as an early check to avoid computing
// `qualifying`.
val qualifying = state.ownedVars -- locked

if (!qualifying.isEmpty) {
typr.println(i"interpolate $tree: ${tree.tpe.widen} in $state, owned vars = ${state.ownedVars.toList}%, %, previous = ${locked.toList}%, % / ${state.constraint}")
val resultAlreadyConstrained =
tree.isInstanceOf[Apply] || tree.tpe.isInstanceOf[MethodOrPoly]
if (!resultAlreadyConstrained)
constrainResult(tree.symbol, tree.tpe, pt)
// This is needed because it could establish singleton type upper bounds. See i2998.scala.

val tp = tree.tpe.widen
val vs = variances(tp)

// Avoid interpolating variables occurring in tree's type if typerstate has unreported errors.
// Reason: The errors might reflect unsatisfiable constraints. In that
// case interpolating without taking account the constraints risks producing
// nonsensical types that then in turn produce incomprehensible errors.
// An example is in neg/i1240.scala. Without the condition in the next code line
// we get for
//
// val y: List[List[String]] = List(List(1))
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: Nothing
// val y: List[List[String]] = List(List(1))
// ^
// With the condition, we get the much more sensical:
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: String
// val y: List[List[String]] = List(List(1))
val hasUnreportedErrors = state.reporter.hasUnreportedErrors
def constraint = state.constraint
for (tvar <- qualifying)
if (!tvar.isInstantiated && state.constraint.contains(tvar)) {
// Needs to be checked again, since previous interpolations could already have
// instantiated `tvar` through unification.
val v = vs(tvar)
if (v == null) {
typr.println(i"interpolate non-occurring $tvar in $state in $tree: $tp, fromBelow = ${tvar.hasLowerBound}, $constraint")
tvar.instantiate(fromBelow = tvar.hasLowerBound)
}
else if (!hasUnreportedErrors)
if (v.intValue != 0) {
typr.println(i"interpolate $tvar in $state in $tree: $tp, fromBelow = ${v.intValue == 1}, $constraint")
tvar.instantiate(fromBelow = v.intValue == 1)

val ownedVars = state.ownedVars
if (ownedVars.size > locked.size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a miracle -- how this change could have such a big impact on performance.

val qualifying = if (locked.isEmpty) ownedVars else ownedVars -- locked
if (!qualifying.isEmpty) {
typr.println(i"interpolate $tree: ${tree.tpe.widen} in $state, owned vars = ${state.ownedVars.toList}%, %, previous = ${locked.toList}%, % / ${state.constraint}")
val resultAlreadyConstrained =
tree.isInstanceOf[Apply] || tree.tpe.isInstanceOf[MethodOrPoly]
if (!resultAlreadyConstrained)
constrainResult(tree.symbol, tree.tpe, pt)
// This is needed because it could establish singleton type upper bounds. See i2998.scala.

val tp = tree.tpe.widen
val vs = variances(tp)

// Avoid interpolating variables occurring in tree's type if typerstate has unreported errors.
// Reason: The errors might reflect unsatisfiable constraints. In that
// case interpolating without taking account the constraints risks producing
// nonsensical types that then in turn produce incomprehensible errors.
// An example is in neg/i1240.scala. Without the condition in the next code line
// we get for
//
// val y: List[List[String]] = List(List(1))
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: Nothing
// val y: List[List[String]] = List(List(1))
// ^
// With the condition, we get the much more sensical:
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: String
// val y: List[List[String]] = List(List(1))
val hasUnreportedErrors = state.reporter.hasUnreportedErrors
def constraint = state.constraint
for (tvar <- qualifying)
if (!tvar.isInstantiated && state.constraint.contains(tvar)) {
// Needs to be checked again, since previous interpolations could already have
// instantiated `tvar` through unification.
val v = vs(tvar)
if (v == null) {
typr.println(i"interpolate non-occurring $tvar in $state in $tree: $tp, fromBelow = ${tvar.hasLowerBound}, $constraint")
tvar.instantiate(fromBelow = tvar.hasLowerBound)
}
else typr.println(i"no interpolation for nonvariant $tvar in $state")
}
else if (!hasUnreportedErrors)
if (v.intValue != 0) {
typr.println(i"interpolate $tvar in $state in $tree: $tp, fromBelow = ${v.intValue == 1}, $constraint")
tvar.instantiate(fromBelow = v.intValue == 1)
}
else typr.println(i"no interpolation for nonvariant $tvar in $state")
}
}
}
tree
}
Expand Down
29 changes: 22 additions & 7 deletions compiler/src/dotty/tools/dotc/util/SimpleIdentitySet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,12 @@ abstract class SimpleIdentitySet[+Elem <: AnyRef] {
def - [E >: Elem <: AnyRef](x: E): SimpleIdentitySet[Elem]
def contains[E >: Elem <: AnyRef](x: E): Boolean
def foreach(f: Elem => Unit): Unit
def toList: List[Elem] = {
val buf = new ListBuffer[Elem]
foreach(buf += _)
buf.toList
}
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A
def toList: List[Elem]
def ++ [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[E] =
((this: SimpleIdentitySet[E]) /: that.toList)(_ + _)
((this: SimpleIdentitySet[E]) /: that)(_ + _)
def -- [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[Elem] =
(this /: that.toList)(_ - _)
(this /: that)(_ - _)
override def toString: String = toList.mkString("(", ", ", ")")
}

Expand All @@ -33,6 +30,8 @@ object SimpleIdentitySet {
this
def contains[E <: AnyRef](x: E): Boolean = false
def foreach(f: Nothing => Unit): Unit = ()
def /: [A, E <: AnyRef](z: A)(f: (A, E) => A): A = z
def toList = Nil
}

private class Set1[+Elem <: AnyRef](x0: AnyRef) extends SimpleIdentitySet[Elem] {
Expand All @@ -43,6 +42,9 @@ object SimpleIdentitySet {
if (x `eq` x0) empty else this
def contains[E >: Elem <: AnyRef](x: E): Boolean = x `eq` x0
def foreach(f: Elem => Unit): Unit = f(x0.asInstanceOf[Elem])
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
f(z, x0.asInstanceOf[E])
def toList = x0.asInstanceOf[Elem] :: Nil
}

private class Set2[+Elem <: AnyRef](x0: AnyRef, x1: AnyRef) extends SimpleIdentitySet[Elem] {
Expand All @@ -55,6 +57,9 @@ object SimpleIdentitySet {
else this
def contains[E >: Elem <: AnyRef](x: E): Boolean = (x `eq` x0) || (x `eq` x1)
def foreach(f: Elem => Unit): Unit = { f(x0.asInstanceOf[Elem]); f(x1.asInstanceOf[Elem]) }
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
f(f(z, x0.asInstanceOf[E]), x1.asInstanceOf[E])
def toList = x0.asInstanceOf[Elem] :: x1.asInstanceOf[Elem] :: Nil
}

private class Set3[+Elem <: AnyRef](x0: AnyRef, x1: AnyRef, x2: AnyRef) extends SimpleIdentitySet[Elem] {
Expand All @@ -78,6 +83,9 @@ object SimpleIdentitySet {
def foreach(f: Elem => Unit): Unit = {
f(x0.asInstanceOf[Elem]); f(x1.asInstanceOf[Elem]); f(x2.asInstanceOf[Elem])
}
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
f(f(f(z, x0.asInstanceOf[E]), x1.asInstanceOf[E]), x2.asInstanceOf[E])
def toList = x0.asInstanceOf[Elem] :: x1.asInstanceOf[Elem] :: x2.asInstanceOf[Elem] :: Nil
}

private class SetN[+Elem <: AnyRef](xs: Array[AnyRef]) extends SimpleIdentitySet[Elem] {
Expand Down Expand Up @@ -115,5 +123,12 @@ object SimpleIdentitySet {
var i = 0
while (i < size) { f(xs(i).asInstanceOf[Elem]); i += 1 }
}
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
(z /: xs.asInstanceOf[Array[E]])(f)
def toList: List[Elem] = {
val buf = new ListBuffer[Elem]
foreach(buf += _)
buf.toList
}
}
}