Skip to content

Fix #4720 and #4721: account for type inference being too smart #4946

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/Positioned.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ abstract class Positioned extends DotClass with Product {
* destructively and the item itself is returned.
*/
def withPos(pos: Position): this.type = {
val newpd = (if (pos == curPos || curPos.isSynthetic) this else clone).asInstanceOf[Positioned]
val newpd = (if (pos == curPos || curPos.isSynthetic) this else clone.asInstanceOf[Positioned])
newpd.setPos(pos)
newpd.asInstanceOf[this.type]
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ object Config {
/** Type comparer will fail with an assert if the upper bound
* of a constrained parameter becomes Nothing. This should be turned
* on only for specific debugging as normally instantiation to Nothing
* is not an error consdition.
* is not an error condition.
*/
final val failOnInstantiationToNothing = false

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Constraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ abstract class Constraint extends Showable {
* are not contained in the return bounds.
* @pre `param` is not part of the constraint domain.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment still correct?

Copy link
Contributor Author

@Blaisorblade Blaisorblade Aug 16, 2018

Choose a reason for hiding this comment

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

I can't make sense of "@pre param is not part of the constraint domain." if that's a precondition. What's the "constraint domain"?

Otherwise, I suspect yes.

Poly params that are known to be smaller or greater than param are not contained in the return bounds.

IIUC, this suggests that in [Y<:String, Z>:Int, W>:Z<:Y], nonParamBounds(Y) and nonParamBounds(Z) don't include W. But nonParamBounds(W) should mention Z and Y, so it might never have been accurate.

EDIT: my patch doesn't affect it, and neither does yours; this comment means, concretely, that we only account for boundsMap but not lowerMap or upperMap. How information is partitioned between those map is another matter entirely (and might be affected by your patch).

def nonParamBounds(param: TypeParamRef): TypeBounds
def nonParamBounds(param: TypeParamRef)(implicit ctx: Context): TypeBounds

/** The lower bound of `param` including all known-to-be-smaller parameters */
def fullLowerBound(param: TypeParamRef)(implicit ctx: Context): Type
Expand Down
29 changes: 16 additions & 13 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -341,19 +341,22 @@ trait ConstraintHandling {
* and propagate all bounds.
* @param tvars See Constraint#add
*/
def addToConstraint(tl: TypeLambda, tvars: List[TypeVar]): Unit =
assert {
checkPropagated(i"initialized $tl") {
constraint = constraint.add(tl, tvars)
tl.paramNames.indices.forall { i =>
val param = tl.paramRefs(i)
val bounds = constraint.nonParamBounds(param)
val lower = constraint.lower(param)
val upper = constraint.upper(param)
if (lower.nonEmpty && !bounds.lo.isRef(defn.NothingClass) ||
upper.nonEmpty && !bounds.hi.isRef(defn.AnyClass)) constr.println(i"INIT*** $tl")
lower.forall(addOneBound(_, bounds.hi, isUpper = true)) &&
upper.forall(addOneBound(_, bounds.lo, isUpper = false))
def addToConstraint(tl: TypeLambda, tvars: List[TypeVar]): Boolean =
checkPropagated(i"initialized $tl") {
constraint = constraint.add(tl, tvars)
tl.paramRefs.forall { param =>
constraint.entry(param) match {
case bounds: TypeBounds =>
val lower = constraint.lower(param)
val upper = constraint.upper(param)
if (lower.nonEmpty && !bounds.lo.isRef(defn.NothingClass) ||
upper.nonEmpty && !bounds.hi.isRef(defn.AnyClass)) constr.println(i"INIT*** $tl")
lower.forall(addOneBound(_, bounds.hi, isUpper = true)) &&
upper.forall(addOneBound(_, bounds.lo, isUpper = false))
case _ =>
// Happens if param was already solved while processing earlier params of the same TypeLambda.
// See #4720.
true
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
def isLess(param1: TypeParamRef, param2: TypeParamRef): Boolean =
upper(param1).contains(param2)

def nonParamBounds(param: TypeParamRef): TypeBounds =
entry(param).asInstanceOf[TypeBounds]
def nonParamBounds(param: TypeParamRef)(implicit ctx: Context): TypeBounds =
entry(param).bounds

def fullLowerBound(param: TypeParamRef)(implicit ctx: Context): Type =
(nonParamBounds(param).lo /: minLower(param))(_ | _)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,10 @@ object ProtoTypes {
s"inconsistent: no typevars were added to committable constraint ${state.constraint}")

def newTypeVars(tl: TypeLambda): List[TypeTree] =
for (n <- (0 until tl.paramNames.length).toList)
for (paramRef <- tl.paramRefs)
yield {
val tt = new TypeVarBinder().withPos(owningTree.pos)
val tvar = new TypeVar(tl.paramRefs(n), state)
val tvar = new TypeVar(paramRef, state)
state.ownedVars += tvar
tt.withType(tvar)
}
Expand Down
4 changes: 4 additions & 0 deletions tests/neg/i4721.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object Test {
def main(args: Array[String]):Unit = m(1) // error
def m[Y<:String, Z>:Int, W>:Z<:Y](d:Y):Unit={}
}
4 changes: 4 additions & 0 deletions tests/neg/i4721a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object Test {
def main(args: Array[String]):Unit = m("1") // error
def m[Y<:String, Z>:Int, W>:Z<:Y](d:Y):Unit={}
}
4 changes: 4 additions & 0 deletions tests/pos/i4720.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object Test {
def main(args: Array[String]):Unit = m(1)
def m[Y<:Int, Z>:Int, W>:Z<:Y](d:Y):Unit={}
}