Skip to content

Fix #2675: Properly handle undetermined type variables in unapply #3889

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
merged 13 commits into from
Feb 1, 2018
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
def TypeBoundsTree(lo: Tree, hi: Tree)(implicit ctx: Context): TypeBoundsTree =
ta.assignType(untpd.TypeBoundsTree(lo, hi), lo, hi)

def Bind(sym: TermSymbol, body: Tree)(implicit ctx: Context): Bind =
def Bind(sym: Symbol, body: Tree)(implicit ctx: Context): Bind =
ta.assignType(untpd.Bind(sym.name, body), sym)

/** A pattern corresponding to `sym: tpe` */
Expand Down
3 changes: 0 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,6 @@ object Flags {
/** A method that has default params */
final val DefaultParameterized = termFlag(27, "<defaultparam>")

/** A type that is defined by a type bind */
final val BindDefinedType = typeFlag(27, "<bind-defined>")

/** Symbol is inlined */
final val Inline = commonFlag(29, "inline")

Expand Down
8 changes: 8 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,14 @@ trait Symbols { this: Context =>
modFlags | PackageCreationFlags, clsFlags | PackageCreationFlags,
Nil, decls)

/** Define a new symbol associated with a Bind or pattern wildcard and
* make it gadt narrowable.
*/
def newPatternBoundSymbol(name: Name, info: Type, pos: Position) = {
val sym = newSymbol(owner, name, Case, info, coord = pos)
if (name.isTypeName) gadt.setBounds(sym, info.bounds)
sym
}

/** Create a stub symbol that will issue a missing reference error
* when attempted to be completed.
Expand Down
24 changes: 7 additions & 17 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1077,23 +1077,13 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
val tparam = tr.symbol
typr.println(i"narrow gadt bound of $tparam: ${tparam.info} from ${if (isUpper) "above" else "below"} to $bound ${bound.isRef(tparam)}")
if (bound.isRef(tparam)) false
else bound match {
case bound: TypeRef
if bound.symbol.is(BindDefinedType) &&
ctx.gadt.bounds.contains(bound.symbol) &&
!tr.symbol.is(BindDefinedType) =>
// Avoid having pattern-bound types in gadt bounds,
// as these might be eliminated once the pattern is typechecked.
// Pattern-bound type symbols should be narrowed first, only if that fails
// should symbols in the environment be constrained.
narrowGADTBounds(bound, tr, !isUpper)
case _ =>
val oldBounds = ctx.gadt.bounds(tparam)
val newBounds =
if (isUpper) TypeBounds(oldBounds.lo, oldBounds.hi & bound)
else TypeBounds(oldBounds.lo | bound, oldBounds.hi)
isSubType(newBounds.lo, newBounds.hi) &&
{ ctx.gadt.setBounds(tparam, newBounds); true }
else {
val oldBounds = ctx.gadt.bounds(tparam)
val newBounds =
if (isUpper) TypeBounds(oldBounds.lo, oldBounds.hi & bound)
else TypeBounds(oldBounds.lo | bound, oldBounds.hi)
isSubType(newBounds.lo, newBounds.hi) &&
{ ctx.gadt.setBounds(tparam, newBounds); true }
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3308,7 +3308,7 @@ object Types {
def isInstantiated(implicit ctx: Context) = instanceOpt.exists

/** Instantiate variable with given type */
private def instantiateWith(tp: Type)(implicit ctx: Context): Type = {
def instantiateWith(tp: Type)(implicit ctx: Context): Type = {
assert(tp ne this, s"self instantiation of ${tp.show}, constraint = ${ctx.typerState.constraint.show}")
typr.println(s"instantiating ${this.show} with ${tp.show}")
if ((ctx.typerState eq owningState.get) && !ctx.typeComparer.subtypeCheckInProgress)
Expand Down
23 changes: 7 additions & 16 deletions compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,6 @@ class TreePickler(pickler: TastyPickler) {
pickleConstant(value)
case tpe: NamedType =>
val sym = tpe.symbol
def pickleDirectRef() = {
writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
pickleSymRef(sym)
}
def pickleExternalRef(sym: Symbol) = {
def pickleCore() = {
pickleNameAndSig(sym.name, tpe.signature)
Expand All @@ -188,17 +184,10 @@ class TreePickler(pickler: TastyPickler) {
writeByte(if (tpe.isType) TYPEREFpkg else TERMREFpkg)
pickleName(sym.fullName)
}
else if (tpe.prefix == NoPrefix)
if (sym is Flags.BindDefinedType) {
registerDef(sym)
writeByte(BIND)
withLength {
pickleName(sym.name)
pickleType(sym.info)
pickleDirectRef()
}
}
else pickleDirectRef()
else if (tpe.prefix == NoPrefix) {
writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
pickleSymRef(sym)
}
else if (isLocallyDefined(sym)) {
writeByte(if (tpe.isType) TYPEREFsymbol else TERMREFsymbol)
pickleSymRef(sym); pickleType(tpe.prefix)
Expand Down Expand Up @@ -445,7 +434,9 @@ class TreePickler(pickler: TastyPickler) {
case Bind(name, body) =>
registerDef(tree.symbol)
writeByte(BIND)
withLength { pickleName(name); pickleType(tree.symbol.info); pickleTree(body) }
withLength {
pickleName(name); pickleType(tree.symbol.info); pickleTree(body)
}
case Alternative(alts) =>
writeByte(ALTERNATIVE)
withLength { alts.foreach(pickleTree) }
Expand Down
81 changes: 60 additions & 21 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ class TreeUnpickler(reader: TastyReader,
while (nextByte == PARAMS || nextByte == TYPEPARAM) skipTree()

/** Record all directly nested definitions and templates in current tree
* as `OwnerTree`s in `buf`
* as `OwnerTree`s in `buf`.
* A complication concerns member definitions. These are lexically nested in a
* Template node, but need to be listed separately in the OwnerTree of the enclosing class
* in order not to confuse owner chains.
*/
def scanTree(buf: ListBuffer[OwnerTree], mode: MemberDefMode = AllDefs): Unit = {
val start = currentAddr
Expand All @@ -118,8 +121,15 @@ class TreeUnpickler(reader: TastyReader,
case VALDEF | DEFDEF | TYPEDEF | TYPEPARAM | PARAM | TEMPLATE =>
val end = readEnd()
for (i <- 0 until numRefs(tag)) readNat()
if (tag == TEMPLATE) scanTrees(buf, end, MemberDefsOnly)
if (mode != NoMemberDefs) buf += new OwnerTree(start, tag, fork, end)
if (tag == TEMPLATE) {
// Read all member definitions now, whereas non-members are children of
// template's owner tree.
val nonMemberReader = fork
scanTrees(buf, end, MemberDefsOnly)
buf += new OwnerTree(start, tag, nonMemberReader, end)
}
else if (mode != NoMemberDefs)
buf += new OwnerTree(start, tag, fork, end)
goto(end)
case tag =>
if (mode == MemberDefsOnly) skipTree(tag)
Expand All @@ -132,7 +142,11 @@ class TreeUnpickler(reader: TastyReader,
}
else {
for (i <- 0 until nrefs) readNat()
scanTrees(buf, end)
if (tag == BIND) {
buf += new OwnerTree(start, tag, fork, end)
goto(end)
}
else scanTrees(buf, end)
}
}
else if (tag >= firstNatASTTreeTag) { readNat(); scanTree(buf) }
Expand Down Expand Up @@ -267,12 +281,6 @@ class TreeUnpickler(reader: TastyReader,
OrType(readType(), readType())
case SUPERtype =>
SuperType(readType(), readType())
case BIND =>
val sym = ctx.newSymbol(ctx.owner, readName().toTypeName, BindDefinedType, readType(),
coord = coordAt(start))
registerSym(start, sym)
if (currentAddr != end) readType()
TypeRef(NoPrefix, sym)
case POLYtype =>
readMethodic(PolyType, _.toTypeName)
case METHODtype =>
Expand Down Expand Up @@ -431,6 +439,8 @@ class TreeUnpickler(reader: TastyReader,
def createSymbol()(implicit ctx: Context): Symbol = nextByte match {
case VALDEF | DEFDEF | TYPEDEF | TYPEPARAM | PARAM =>
createMemberSymbol()
case BIND =>
createBindSymbol()
case TEMPLATE =>
val localDummy = ctx.newLocalDummy(ctx.owner)
registerSym(currentAddr, localDummy)
Expand All @@ -439,6 +449,25 @@ class TreeUnpickler(reader: TastyReader,
throw new Error(s"illegal createSymbol at $currentAddr, tag = $tag")
}

private def createBindSymbol()(implicit ctx: Context): Symbol = {
val start = currentAddr
val tag = readByte()
val end = readEnd()
var name: Name = readName()
nextUnsharedTag match {
case TYPEBOUNDS | TYPEALIAS => name = name.toTypeName
case _ =>
}
val typeReader = fork
val completer = new LazyType {
def complete(denot: SymDenotation)(implicit ctx: Context) =
denot.info = typeReader.readType()
}
val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, completer, coord = coordAt(start))
registerSym(start, sym)
sym
}

/** Create symbol of member definition or parameter node and enter in symAtAddr map
* @return the created symbol
*/
Expand Down Expand Up @@ -994,10 +1023,9 @@ class TreeUnpickler(reader: TastyReader,
val elemtpt = readTpt()
SeqLiteral(until(end)(readTerm()), elemtpt)
case BIND =>
val name = readName()
val info = readType()
val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, info, coord = coordAt(start))
registerSym(start, sym)
val sym = symAtAddr.getOrElse(start, forkAt(start).createSymbol())
readName()
readType()
Bind(sym, readTerm())
case ALTERNATIVE =>
Alternative(until(end)(readTerm()))
Expand Down Expand Up @@ -1152,11 +1180,16 @@ class TreeUnpickler(reader: TastyReader,
*/
class OwnerTree(val addr: Addr, tag: Int, reader: TreeReader, val end: Addr) {

private var myChildren: List[OwnerTree] = null

/** All definitions that have the definition at `addr` as closest enclosing definition */
lazy val children: List[OwnerTree] = {
val buf = new ListBuffer[OwnerTree]
reader.scanTrees(buf, end, if (tag == TEMPLATE) NoMemberDefs else AllDefs)
buf.toList
def children: List[OwnerTree] = {
if (myChildren == null) myChildren = {
val buf = new ListBuffer[OwnerTree]
reader.scanTrees(buf, end, if (tag == TEMPLATE) NoMemberDefs else AllDefs)
buf.toList
}
myChildren
}

/** Find the owner of definition at `addr` */
Expand All @@ -1175,13 +1208,19 @@ class TreeUnpickler(reader: TastyReader,
}
catch {
case ex: TreeWithoutOwner =>
println(i"no owner for $addr among $cs") // DEBUG
pickling.println(i"no owner for $addr among $cs%, %")
throw ex
}
try search(children, NoSymbol)
catch {
case ex: TreeWithoutOwner =>
pickling.println(s"ownerTree = $ownerTree")
throw ex
}
search(children, NoSymbol)
}

override def toString = s"OwnerTree(${addr.index}, ${end.index}"
override def toString =
s"OwnerTree(${addr.index}, ${end.index}, ${if (myChildren == null) "?" else myChildren.mkString(" ")})"
}
}

Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/TailRec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ class TailRec extends MiniPhase with FullParameterization {
val isRecursiveCall = (method eq sym)
val recvWiden = prefix.tpe.widenDealias


def continue = {
val method = noTailTransform(call)
val methodWithTargs = if (targs.nonEmpty) TypeApply(method, targs) else method
Expand All @@ -237,7 +236,9 @@ class TailRec extends MiniPhase with FullParameterization {

if (isRecursiveCall) {
if (ctx.tailPos) {
val receiverIsSame = enclosingClass.appliedRef.widenDealias =:= recvWiden
val receiverIsSame =
recvWiden <:< enclosingClass.appliedRef &&
(sym.isEffectivelyFinal || enclosingClass.appliedRef <:< recvWiden)
val receiverIsThis = prefix.tpe =:= thisType || prefix.tpe.widen =:= thisType

def rewriteTailCall(recv: Tree): Tree = {
Expand Down
39 changes: 17 additions & 22 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -890,12 +890,26 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
/** Produce a typed qual.unapply or qual.unapplySeq tree, or
* else if this fails follow a type alias and try again.
*/
val unapplyFn = trySelectUnapply(qual) { sel =>
var unapplyFn = trySelectUnapply(qual) { sel =>
val qual1 = followTypeAlias(qual)
if (qual1.isEmpty) notAnExtractor(sel)
else trySelectUnapply(qual1)(_ => notAnExtractor(sel))
}

/** Add a `Bind` node for each `bound` symbol in a type application `unapp` */
def addBinders(unapp: Tree, bound: List[Symbol]) = unapp match {
case TypeApply(fn, args) =>
def addBinder(arg: Tree) = arg.tpe.stripTypeVar match {
case ref: TypeRef if bound.contains(ref.symbol) =>
tpd.Bind(ref.symbol, Ident(ref))
case _ =>
arg
}
tpd.cpy.TypeApply(unapp)(fn, args.mapConserve(addBinder))
case _ =>
unapp
}

def fromScala2x = unapplyFn.symbol.exists && (unapplyFn.symbol.owner is Scala2x)

/** Is `subtp` a subtype of `tp` or of some generalization of `tp`?
Expand All @@ -922,27 +936,8 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
unapp.println(i"case 1 $unapplyArgType ${ctx.typerState.constraint}")
selType
} else if (isSubTypeOfParent(unapplyArgType, selType)(ctx.addMode(Mode.GADTflexible))) {
maximizeType(unapplyArgType) match {
case Some(tvar) =>
def msg =
ex"""There is no best instantiation of pattern type $unapplyArgType
|that makes it a subtype of selector type $selType.
|Non-variant type variable ${tvar.origin} cannot be uniquely instantiated."""
if (fromScala2x) {
// We can't issue an error here, because in Scala 2, ::[B] is invariant
// whereas List[+T] is covariant. According to the strict rule, a pattern
// match of a List[C] against a case x :: xs is illegal, because
// B cannot be uniquely instantiated. Of course :: should have been
// covariant in the first place, but in the Scala libraries it isn't.
// So for now we allow these kinds of patterns, even though they
// can open unsoundness holes. See SI-7952 for an example of the hole this opens.
if (ctx.settings.verbose.value) ctx.warning(msg, tree.pos)
} else {
unapp.println(s" ${unapplyFn.symbol.owner} ${unapplyFn.symbol.owner is Scala2x}")
ctx.strictWarning(msg, tree.pos)
}
case _ =>
}
val patternBound = maximizeType(unapplyArgType, tree.pos, fromScala2x)
if (patternBound.nonEmpty) unapplyFn = addBinders(unapplyFn, patternBound)
unapp.println(i"case 2 $unapplyArgType ${ctx.typerState.constraint}")
unapplyArgType
} else {
Expand Down
22 changes: 14 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/Inferencing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Trees._
import Constants._
import Scopes._
import ProtoTypes._
import NameKinds.WildcardParamName
import annotation.unchecked
import util.Positions._
import util.{Stats, SimpleIdentityMap}
Expand Down Expand Up @@ -222,23 +223,28 @@ object Inferencing {
case _ => NoType
}

/** Instantiate undetermined type variables to that type `tp` is
* maximized and return None. If this is not possible, because a non-variant
* typevar is not uniquely determined, return that typevar in a Some.
/** Instantiate undetermined type variables so that type `tp` is maximized.
* @return The list of type symbols that were created
* to instantiate undetermined type variables that occur non-variantly
*/
def maximizeType(tp: Type)(implicit ctx: Context): Option[TypeVar] = Stats.track("maximizeType") {
def maximizeType(tp: Type, pos: Position, fromScala2x: Boolean)(implicit ctx: Context): List[Symbol] = Stats.track("maximizeType") {
val vs = variances(tp, alwaysTrue)
var result: Option[TypeVar] = None
val patternBound = new mutable.ListBuffer[Symbol]
vs foreachBinding { (tvar, v) =>
if (v == 1) tvar.instantiate(fromBelow = false)
else if (v == -1) tvar.instantiate(fromBelow = true)
else {
val bounds = ctx.typerState.constraint.fullBounds(tvar.origin)
if (!(bounds.hi <:< bounds.lo)) result = Some(tvar)
tvar.instantiate(fromBelow = false)
if (bounds.hi <:< bounds.lo || bounds.hi.classSymbol.is(Final) || fromScala2x)
tvar.instantiate(fromBelow = false)
else {
val wildCard = ctx.newPatternBoundSymbol(WildcardParamName.fresh().toTypeName, bounds, pos)
tvar.instantiateWith(wildCard.typeRef)
patternBound += wildCard
}
}
}
result
patternBound.toList
}

type VarianceMap = SimpleIdentityMap[TypeVar, Integer]
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ trait TypeAssigner {
def assignType(tree: untpd.Apply, fn: Tree, args: List[Tree])(implicit ctx: Context) = {
val ownType = fn.tpe.widen match {
case fntpe: MethodType =>
if (sameLength(fntpe.paramInfos, args) || ctx.phase.prev.relaxedTyping || true)
if (sameLength(fntpe.paramInfos, args) || ctx.phase.prev.relaxedTyping)
if (fntpe.isDependent) safeSubstParams(fntpe.resultType, fntpe.paramRefs, args.tpes)
else fntpe.resultType
else
Expand Down
Loading