Skip to content

Fixes to transparent to enable generic tuples #4916

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 9 commits into from
Aug 22, 2018
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
101 changes: 88 additions & 13 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import Symbols._
import Types._
import Decorators._
import Constants._
import StdNames.nme
import StdNames._
import Contexts.Context
import Names.{Name, TermName, EmptyTermName}
import NameOps._
Expand Down Expand Up @@ -602,7 +602,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
newOwners = ctx.owner :: Nil,
substFrom = ddef.vparamss.head.map(_.symbol),
substTo = argSyms)
Block(bindingsBuf.toList, expander.transform(ddef.rhs))
seq(bindingsBuf.toList, expander.transform(ddef.rhs))
case _ => tree
}
case _ => tree
Expand All @@ -629,7 +629,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
/** Try to match pattern `pat` against scrutinee reference `scrut`. If successful add
* bindings for variables bound in this pattern to `bindingsBuf`.
*/
def reducePattern(bindingsBuf: mutable.ListBuffer[MemberDef], scrut: TermRef, pat: Tree): Boolean = {
def reducePattern(bindingsBuf: mutable.ListBuffer[MemberDef], scrut: TermRef, pat: Tree)(implicit ctx: Context): Boolean = {
val isImplicit = scrut.info == defn.ImplicitScrutineeTypeRef

def newBinding(name: TermName, flags: FlagSet, rhs: Tree): Symbol = {
Expand All @@ -655,8 +655,32 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {

pat match {
case Typed(pat1, tpt) =>
val getBoundVars = new TreeAccumulator[List[TypeSymbol]] {
def apply(syms: List[TypeSymbol], t: Tree)(implicit ctx: Context) = {
val syms1 = t match {
case t: Bind if t.symbol.isType && t.name != tpnme.WILDCARD =>
t.symbol.asType :: syms
case _ =>
syms
}
foldOver(syms1, t)
}
}
val boundVars = getBoundVars(Nil, tpt)
for (bv <- boundVars) ctx.gadt.setBounds(bv, bv.info.bounds)
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels a bit ad hoc, isn't there a way to reuse normal type checking of patterns instead of redoing all that work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, but did not find a good way to re-use what's there. it's spread out over several different things.

if (isImplicit) searchImplicit(nme.WILDCARD, tpt)
else scrut <:< tpt.tpe && reducePattern(bindingsBuf, scrut, pat1)
else scrut <:< tpt.tpe && {
for (bv <- boundVars) {
bv.info = TypeAlias(ctx.gadt.bounds(bv).lo)
// FIXME: This is very crude. We should approximate with lower or higher bound depending
// on variance, and we should also take care of recursive bounds. Basically what
// ConstraintHandler#approximation does. However, this only works for constrained paramrefs
// not GADT-bound variables. Hopefully we will get some way to improve this when we
// re-implement GADTs in terms of constraints.
bindingsBuf += TypeDef(bv)
}
reducePattern(bindingsBuf, scrut, pat1)
}
case pat @ Bind(name: TermName, Typed(_, tpt)) if isImplicit =>
searchImplicit(name, tpt)
case pat @ Bind(name: TermName, body) =>
Expand Down Expand Up @@ -706,16 +730,19 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
val scrutineeBinding = normalizeBinding(ValDef(scrutineeSym, scrutinee))

def reduceCase(cdef: untpd.CaseDef): MatchRedux = {
def guardOK = cdef.guard.isEmpty || {
typer.typed(cdef.guard, defn.BooleanType) match {
val caseBindingsBuf = new mutable.ListBuffer[MemberDef]()
def guardOK(implicit ctx: Context) = cdef.guard.isEmpty || {
val guardCtx = ctx.fresh.setNewScope
caseBindingsBuf.foreach(binding => guardCtx.enter(binding.symbol))
typer.typed(cdef.guard, defn.BooleanType)(guardCtx) match {
case ConstantValue(true) => true
case _ => false
}
}
val caseBindingsBuf = new mutable.ListBuffer[MemberDef]()
if (scrutType != defn.ImplicitScrutineeTypeRef) caseBindingsBuf += scrutineeBinding
val pat1 = typer.typedPattern(cdef.pat, scrutType)(typer.gadtContext(gadtSyms))
if (reducePattern(caseBindingsBuf, scrutineeSym.termRef, pat1) && guardOK)
val gadtCtx = typer.gadtContext(gadtSyms).addMode(Mode.GADTflexible)
val pat1 = typer.typedPattern(cdef.pat, scrutType)(gadtCtx)
if (reducePattern(caseBindingsBuf, scrutineeSym.termRef, pat1)(gadtCtx) && guardOK)
Some((caseBindingsBuf.toList, cdef.body))
else
None
Expand Down Expand Up @@ -809,6 +836,8 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
def dropUnusedDefs(bindings: List[MemberDef], tree: Tree)(implicit ctx: Context): (List[MemberDef], Tree) = {
val refCount = newMutableSymbolMap[Int]
val bindingOfSym = newMutableSymbolMap[MemberDef]
val dealiased = new java.util.IdentityHashMap[Type, Type]()

def isInlineable(binding: MemberDef) = binding match {
case DefDef(_, Nil, Nil, _, _) => true
case vdef @ ValDef(_, _, _) => isPureExpr(vdef.rhs)
Expand All @@ -818,6 +847,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
refCount(binding.symbol) = 0
bindingOfSym(binding.symbol) = binding
}

val countRefs = new TreeTraverser {
override def traverse(t: Tree)(implicit ctx: Context) = {
def updateRefCount(sym: Symbol, inc: Int) =
Expand All @@ -844,6 +874,45 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
}
} && !boundSym.is(TransparentImplicitMethod)

val (termBindings, typeBindings) = bindings.partition(_.symbol.isTerm)

/** drop any referenced type symbols from the given set of type symbols */
val dealiasTypeBindings = new TreeMap {
val boundTypes = typeBindings.map(_.symbol).toSet

val dealias = new TypeMap {
override def apply(tp: Type) = dealiased.get(tp) match {
case null =>
val tp1 = mapOver {
tp match {
case tp: TypeRef if boundTypes.contains(tp.symbol) =>
val TypeAlias(alias) = tp.info
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we get anything other than a TypeAlias here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should always be a TypeAlias because of the way we generate type bindings. So boundTypes.contains(tp.symbol) should imply tp.info is a TypeAlias.

alias
case _ => tp
}
}
dealiased.put(tp, tp1)
tp1
case tp1 => tp1
}
}

override def transform(t: Tree)(implicit ctx: Context) = {
val dealiasedType = dealias(t.tpe)
val t1 = t match {
case t: RefTree =>
if (boundTypes.contains(t.symbol)) TypeTree(dealiasedType).withPos(t.pos)
else t.withType(dealiasedType)
case t: DefTree =>
t.symbol.info = dealias(t.symbol.info)
t
case _ =>
t.withType(dealiasedType)
}
super.transform(t1)
}
}

val inlineBindings = new TreeMap {
override def transform(t: Tree)(implicit ctx: Context) = t match {
case t: RefTree =>
Expand All @@ -859,17 +928,23 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
case t: Apply =>
val t1 = super.transform(t)
if (t1 `eq` t) t else reducer.betaReduce(t1)
case Block(Nil, expr) =>
super.transform(expr)
case _ =>
super.transform(t)
}
}

val retained = bindings.filterConserve(binding => retain(binding.symbol))
if (retained `eq` bindings) {
(bindings, tree)
val dealiasedTermBindings =
termBindings.mapconserve(dealiasTypeBindings.transform).asInstanceOf[List[MemberDef]]
val dealiasedTree = dealiasTypeBindings.transform(tree)

val retained = dealiasedTermBindings.filterConserve(binding => retain(binding.symbol))
if (retained `eq` dealiasedTermBindings) {
(dealiasedTermBindings, dealiasedTree)
}
else {
val expanded = inlineBindings.transform(tree)
val expanded = inlineBindings.transform(dealiasedTree)
dropUnusedDefs(retained, expanded)
}
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/PrepareTransparent.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ object PrepareTransparent {
def markTopLevelMatches(meth: Symbol, tree: untpd.Tree)(implicit ctx: Context): Unit = tree match {
case tree: untpd.Match =>
tree.putAttachment(TopLevelMatch, ())
tree.cases.foreach(markTopLevelMatches(meth, _))
for (cdef <- tree.cases) markTopLevelMatches(meth, cdef.body)
case tree: untpd.Block =>
markTopLevelMatches(meth, tree.expr)
case _ =>
Expand All @@ -76,12 +76,14 @@ object PrepareTransparent {
* by excluding all symbols properly contained in the inlined method.
*
* Constant vals don't need accessors since they are inlined in FirstTransform.
* Transparent methods don't need accessors since they are inlined in Typer.
*/
def needsAccessor(sym: Symbol)(implicit ctx: Context) =
sym.isTerm &&
(sym.is(AccessFlags) || sym.privateWithin.exists) &&
!sym.isContainedIn(inlineSym) &&
!(sym.isStable && sym.info.widenTermRefExpr.isInstanceOf[ConstantType])
!(sym.isStable && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
!sym.is(TransparentMethod)

def preTransform(tree: Tree)(implicit ctx: Context): Tree

Expand Down
4 changes: 0 additions & 4 deletions compiler/src/dotty/tools/dotc/util/Set.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
/* NSC -- new Scala compiler
* Copyright 2005-2012 LAMP/EPFL
* @author Martin Odersky
*/
package dotty.tools.dotc.util

/** A common class for lightweight sets.
Expand Down
34 changes: 34 additions & 0 deletions tests/run/TupleAbstract.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
()
(1)
(A,1)
(2,A,1)
(B,2,A,1)
(3,B,2,A,1)
(C,3,B,2,A,1)
(4,C,3,B,2,A,1)
(D,4,C,3,B,2,A,1)
h1 = 1
h2 = A
h7 = 4
h8 = D
t1 = ()
t2 = (1)
t7 = (C,3,B,2,A,1)
t8 = (4,C,3,B,2,A,1)
a1_0 = 1
a2_0 = A
a3_1 = A
a4_3 = 1
a6_4 = A
a8_0 = D
c0_0 = ()
c0_1 = (1)
c1_0 = (1)
c0_4 = (B,2,A,1)
c4_0 = (B,2,A,1)
c1_1 = (1,1)
c1_8 = (1,D,4,C,3,B,2,A,1)
c2_1 = (A,1,1)
c2_2 = (A,1,A,1)
c2_3 = (A,1,2,A,1)
c3_3 = (2,A,1,2,A,1)
Loading