Skip to content

Commit

Permalink
Fix issue with higher-order type params.
Browse files Browse the repository at this point in the history
I think I found an issue underlying more than one bit of sketchy
behavior amongst CC[_] and friends.

Plus, I managed to initialize TypeConstraints with the bounds of the
originating type parameter. I feel like that should cause something
nifty to happen somewhere, but I have seen neither confetti nor lasers
in greater quantities than I usually do. Will keep my remaining eye out.

Closes SI-5359, review by @moors.
  • Loading branch information
paulp committed Jan 6, 2012
1 parent 020053c commit cdd4aac
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/compiler/scala/reflect/internal/Symbols.scala
Expand Up @@ -409,7 +409,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
final def isError = hasFlag(IS_ERROR)
final def isErroneous = isError || isInitialized && tpe.isErroneous
final def isTypeParameterOrSkolem = isType && hasFlag(PARAM)
final def isHigherOrderTypeParameter = owner.isTypeParameterOrSkolem
final def isHigherOrderTypeParameter = (this ne NoSymbol) && owner.isTypeParameterOrSkolem
final def isTypeSkolem = isSkolem && hasFlag(PARAM)
// a type symbol bound by an existential type, for instance the T in
// List[T] forSome { type T }
Expand Down
45 changes: 33 additions & 12 deletions src/compiler/scala/reflect/internal/Types.scala
Expand Up @@ -2451,9 +2451,29 @@ A type's typeSymbol should never be inspected directly.
*/
def unapply(tv: TypeVar): Some[(Type, TypeConstraint)] = Some((tv.origin, tv.constr))
def apply(origin: Type, constr: TypeConstraint) = new TypeVar(origin, constr, List(), List())
// TODO why not initialise TypeConstraint with bounds of tparam?
// @PP: I tried that, didn't work out so well for me.
def apply(tparam: Symbol) = new TypeVar(tparam.tpeHK, new TypeConstraint, List(), tparam.typeParams)
// See pos/tcpoly_infer_implicit_tuple_wrapper for the test which
// fails if I initialize the type constraint with the type parameter
// bounds. It seems that in that instance it interferes with the
// inference. Thus, the isHigherOrderTypeParameter condition.
def apply(tparam: Symbol) = {
val constr = (
if (tparam.isAbstractType && tparam.typeParams.nonEmpty) {
// Force the info of a higher-order tparam's parameters.
// Otherwise things don't end well. See SI-5359.
val info = tparam.info
if (info.bounds exists (t => t.typeSymbol.isHigherOrderTypeParameter)) {
log("TVar(" + tparam + ") receives empty constraint due to higher order type parameter in bounds " + info.bounds)
new TypeConstraint
}
else {
log("TVar(" + tparam + ") constraint initialized with bounds " + info.bounds)
new TypeConstraint(info.bounds)
}
}
else new TypeConstraint

This comment has been minimized.

Copy link
@adriaanm

adriaanm Jan 6, 2012

Contributor

why does this case not initialize the constraint with info.bounds?

This comment has been minimized.

Copy link
@paulp

paulp Jan 6, 2012

Author Contributor

I forget. I feel like I was seeing things I didn't expect coming through that method and I narrowed it down to things where I expected info.bounds to make sense. I'll look at it again.

This comment has been minimized.

Copy link
@paulp

paulp Jan 7, 2012

Author Contributor

Now I remember. There is an explosion of bootstrap-destroying cycles if one attempts this in the general case. I wasn't able to make it work no matter what I did to restrict things.

)
new TypeVar(tparam.tpeHK, constr, Nil, tparam.typeParams)
}
def apply(origin: Type, constr: TypeConstraint, args: List[Type], params: List[Symbol]) =
new TypeVar(origin, constr, args, params)
}
Expand Down Expand Up @@ -2495,10 +2515,9 @@ A type's typeSymbol should never be inspected directly.
override val typeArgs: List[Type],
override val params: List[Symbol]
) extends Type {
private val numArgs = typeArgs.length
// params are needed to keep track of variance (see mapOverArgs in SubstMap)
assert(typeArgs.isEmpty || sameLength(typeArgs, params))
// var tid = { tidCount += 1; tidCount } //DEBUG
assert(typeArgs.isEmpty || sameLength(typeArgs, params),
"%s / params=%s / args=%s".format(origin, params, typeArgs))

/** The constraint associated with the variable */
var constr = constr0
Expand Down Expand Up @@ -2740,11 +2759,12 @@ A type's typeSymbol should never be inspected directly.
override def isVolatile = origin.isVolatile

private def levelString = if (settings.explaintypes.value) level else ""
override def safeToString = constr.inst match {
case null => "<null " + origin + ">"
case NoType => "?" + levelString + origin + typeArgsString(this)
case x => "" + x
}
override def safeToString = (
if (constr eq null) "TVar<%s,constr=null>".format(origin)
else if (constr.inst eq null) "TVar<%s,constr.inst=null>".format(origin)
else if (constr.inst eq NoType) "?" + levelString + origin + typeArgsString(this)
else "" + constr.inst
)
override def kind = "TypeVar"

def cloneInternal = {
Expand Down Expand Up @@ -3248,6 +3268,7 @@ A type's typeSymbol should never be inspected directly.
*/
class TypeConstraint(lo0: List[Type], hi0: List[Type], numlo0: Type, numhi0: Type, avoidWidening0: Boolean = false) {
def this(lo0: List[Type], hi0: List[Type]) = this(lo0, hi0, NoType, NoType)
def this(bounds: TypeBounds) = this(List(bounds.lo), List(bounds.hi))
def this() = this(List(), List())

private var lobounds = lo0
Expand Down Expand Up @@ -4159,7 +4180,7 @@ A type's typeSymbol should never be inspected directly.
case WildcardType =>
TypeVar(tp, new TypeConstraint)
case BoundedWildcardType(bounds) =>
TypeVar(tp, new TypeConstraint(List(bounds.lo), List(bounds.hi)))
TypeVar(tp, new TypeConstraint(bounds))
case _ =>
mapOver(tp)
}
Expand Down
17 changes: 17 additions & 0 deletions test/files/pos/t5359.scala
@@ -0,0 +1,17 @@
// /scala/trac/5359/a.scala
// Thu Jan 5 13:31:05 PST 2012

object test {
trait Step[F[_]] {
// crash: typeConstructor inapplicable for <none>
this match {
case S1() =>
}
}
case class S1[F[_]]() extends Step[F]

// okay
(null: Step[Option]) match {
case S1() =>
}
}

0 comments on commit cdd4aac

Please sign in to comment.