Skip to content

Fix #4466: Update contexts of FunProto when something else is tried #4871

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 6 commits into from
Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ trait TreeInfo[T >: Untyped <: Type] { self: Trees.Instance[T] =>
case _ => tree
}

/** If this is a block, its expression part */
def stripBlock(tree: Tree): Tree = unsplice(tree) match {
case Block(_, expr) => stripBlock(expr)
case _ => tree
}

/** The number of arguments in an application */
def numArgs(tree: Tree): Int = unsplice(tree) match {
case Apply(fn, args) => numArgs(fn) + args.length
Expand Down
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 @@ -981,7 +981,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {

def applyOverloaded(receiver: Tree, method: TermName, args: List[Tree], targs: List[Type], expectedType: Type, isAnnotConstructor: Boolean = false)(implicit ctx: Context): Tree = {
val typer = ctx.typer
val proto = new FunProtoTyped(args, expectedType, typer)
val proto = new FunProtoTyped(args, expectedType)(typer)
val denot = receiver.tpe.member(method)
assert(denot.exists, i"no member $receiver . $method, members = ${receiver.tpe.decls}")
val selected =
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1593,6 +1593,7 @@ object Types {
def isMatchedBy(tp: Type)(implicit ctx: Context): Boolean
def fold[T](x: T, ta: TypeAccumulator[T])(implicit ctx: Context): T
def map(tm: TypeMap)(implicit ctx: Context): ProtoType
def withContext(ctx: Context): ProtoType = this
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a documentation comment.

}

/** Implementations of this trait cache the results of `narrow`. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ class TreeUnpickler(reader: TastyReader,
val vparams = readIndexedParams[ValDef](PARAM)
val parents = collectWhile(nextByte != SELFDEF && nextByte != DEFDEF) {
nextUnsharedTag match {
case APPLY | TYPEAPPLY => readTerm()(parentCtx)
case APPLY | TYPEAPPLY | BLOCK => readTerm()(parentCtx)
case _ => readTpt()(parentCtx)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
": " ~ toText(tp.memberProto) ~ " }"
case tp: ViewProto =>
return toText(tp.argType) ~ " ?=>? " ~ toText(tp.resultType)
case tp @ FunProto(args, resultType, _) =>
case tp @ FunProto(args, resultType) =>
val argsText = args match {
case dummyTreeOfType(tp) :: Nil if !(tp isRef defn.NullClass) => "null: " ~ toText(tp)
case _ => toTextGlobal(args, ", ")
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ object Erasure {
val Apply(fun, args) = tree
if (fun.symbol == defn.cbnArg)
typedUnadapted(args.head, pt)
else typedExpr(fun, FunProto(args, pt, this)) match {
else typedExpr(fun, FunProto(args, pt)(this)) match {
case fun1: Apply => // arguments passed in prototype were already passed
fun1
case fun1 =>
Expand Down
16 changes: 10 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/Mixin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,19 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>
* to be used as initializers of trait parameters if the target of the call
* is a trait.
*/
def transformConstructor(tree: Tree): (Tree, List[Tree]) = {
val Apply(sel @ Select(New(_), nme.CONSTRUCTOR), args) = tree
val (callArgs, initArgs) = if (tree.symbol.owner.is(Trait)) (Nil, args) else (args, Nil)
(superRef(tree.symbol, tree.pos).appliedToArgs(callArgs), initArgs)
def transformConstructor(tree: Tree): (Tree, List[Tree]) = tree match {
case Block(stats, expr) =>
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, if this works, doesn't it mean that we could very easily support early initializers (for the Scala 2 mode) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's still a way to go if we want to bring back early initializers. We'd also have to map definitions in the block back to definitions of the class fields.

val (scall, inits) = transformConstructor(expr)
(cpy.Block(tree)(stats, scall), inits)
case _ =>
val Apply(sel @ Select(New(_), nme.CONSTRUCTOR), args) = tree
val (callArgs, initArgs) = if (tree.symbol.owner.is(Trait)) (Nil, args) else (args, Nil)
(superRef(tree.symbol, tree.pos).appliedToArgs(callArgs), initArgs)
}

val superCallsAndArgs = (
for (p <- impl.parents if p.symbol.isConstructor)
yield p.symbol.owner -> transformConstructor(p)
for (p <- impl.parents; constr = stripBlock(p).symbol if constr.isConstructor)
yield constr.owner -> transformConstructor(p)
).toMap
val superCalls = superCallsAndArgs.mapValues(_._1)
val initArgs = superCallsAndArgs.mapValues(_._2)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
def typedApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = {

def realApply(implicit ctx: Context): Tree = track("realApply") {
val originalProto = new FunProto(tree.args, IgnoredProto(pt), this)(argCtx(tree))
val originalProto = new FunProto(tree.args, IgnoredProto(pt))(this)(argCtx(tree))
val fun1 = typedExpr(tree.fun, originalProto)

// Warning: The following lines are dirty and fragile. We record that auto-tupling was demanded as
Expand Down Expand Up @@ -1365,7 +1365,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
alts filter (isApplicable(_, argTypes, resultType))

val candidates = pt match {
case pt @ FunProto(args, resultType, _) =>
case pt @ FunProto(args, resultType) =>
val numArgs = args.length
val normArgs = args.mapConserve {
case Block(Nil, expr) => expr
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@ trait Implicits { self: Typer =>

lazy val funProto = fullProto match {
case proto: ViewProto =>
FunProto(untpd.TypedSplice(dummyTreeOfType(proto.argType)) :: Nil, proto.resultType, self)
FunProto(untpd.TypedSplice(dummyTreeOfType(proto.argType)) :: Nil, proto.resultType)(self)
case proto => proto
}

Expand Down
78 changes: 44 additions & 34 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -210,28 +210,38 @@ object ProtoTypes {

trait ApplyingProto extends ProtoType

class FunProtoState {

/** The list of typed arguments, if all arguments are typed */
var typedArgs: List[Tree] = Nil

/** A map in which typed arguments can be stored to be later integrated in `typedArgs`. */
var typedArg: SimpleIdentityMap[untpd.Tree, Tree] = SimpleIdentityMap.Empty

/** A map recording the typer states and constraints in which arguments stored in myTypedArg were typed */
var evalState: SimpleIdentityMap[untpd.Tree, (TyperState, Constraint)] = SimpleIdentityMap.Empty

/** The tupled version of this prototype, if it has been computed */
var tupled: Type = NoType

/** If true, the application of this prototype was canceled. */
var toDrop: Boolean = false
}

/** A prototype for expressions that appear in function position
*
* [](args): resultType
*/
case class FunProto(args: List[untpd.Tree], resType: Type, typer: Typer)(implicit ctx: Context)
case class FunProto(args: List[untpd.Tree], resType: Type)(typer: Typer, state: FunProtoState = new FunProtoState)(implicit ctx: Context)
extends UncachedGroundType with ApplyingProto {
private[this] var myTypedArgs: List[Tree] = Nil

override def resultType(implicit ctx: Context) = resType

/** A map in which typed arguments can be stored to be later integrated in `typedArgs`. */
private[this] var myTypedArg: SimpleIdentityMap[untpd.Tree, Tree] = SimpleIdentityMap.Empty

/** A map recording the typer states and constraints in which arguments stored in myTypedArg were typed */
private[this] var evalState: SimpleIdentityMap[untpd.Tree, (TyperState, Constraint)] = SimpleIdentityMap.Empty

def isMatchedBy(tp: Type)(implicit ctx: Context) =
typer.isApplicable(tp, Nil, unforcedTypedArgs, resultType)

def derivedFunProto(args: List[untpd.Tree] = this.args, resultType: Type, typer: Typer = this.typer) =
if ((args eq this.args) && (resultType eq this.resultType) && (typer eq this.typer)) this
else new FunProto(args, resultType, typer)
else new FunProto(args, resultType)(typer)

override def notApplied = WildcardType

Expand All @@ -244,19 +254,19 @@ object ProtoTypes {
* @return True if all arguments have types (in particular, no types were forgotten).
*/
def allArgTypesAreCurrent()(implicit ctx: Context): Boolean = {
evalState foreachBinding { (arg, tstateConstr) =>
state.evalState foreachBinding { (arg, tstateConstr) =>
if ((tstateConstr._1.uncommittedAncestor.constraint `ne` ctx.typerState.constraint) ||
tstateConstr._2.isRetracted) {
typr.println(i"need to invalidate $arg / ${myTypedArg(arg)}, ${tstateConstr._2}, current = ${ctx.typerState.constraint}")
myTypedArg = myTypedArg.remove(arg)
evalState = evalState.remove(arg)
typr.println(i"need to invalidate $arg / ${state.typedArg(arg)}, ${tstateConstr._2}, current = ${ctx.typerState.constraint}")
state.typedArg = state.typedArg.remove(arg)
state.evalState = state.evalState.remove(arg)
Copy link
Member

Choose a reason for hiding this comment

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

This wasn't done before either, but shouldn't state.typedArgs be reset to an empty map too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. But I only wanted to do the minimum change here. We have to come back to this at some point.

}
}
myTypedArg.size == args.length
state.typedArg.size == args.length
}

private def cacheTypedArg(arg: untpd.Tree, typerFn: untpd.Tree => Tree, force: Boolean)(implicit ctx: Context): Tree = {
var targ = myTypedArg(arg)
var targ = state.typedArg(arg)
if (targ == null) {
if (!force && untpd.functionWithUnknownParamType(arg).isDefined)
// If force = false, assume ? rather than reporting an error.
Expand All @@ -270,8 +280,8 @@ object ProtoTypes {
// typerstate if there are no errors. If we also omitted the next two lines
// when warning were emitted, `pos/t1756.scala` would fail when run with -feature.
// It would produce an orphan type parameter for CI when pickling.
myTypedArg = myTypedArg.updated(arg, targ)
evalState = evalState.updated(arg, (ctx.typerState, ctx.typerState.constraint))
state.typedArg = state.typedArg.updated(arg, targ)
state.evalState = state.evalState.updated(arg, (ctx.typerState, ctx.typerState.constraint))
}
}
}
Expand All @@ -285,9 +295,9 @@ object ProtoTypes {
* "missing parameter type" error
*/
private def typedArgs(force: Boolean): List[Tree] = {
if (myTypedArgs.size != args.length)
myTypedArgs = args.mapconserve(cacheTypedArg(_, typer.typed(_), force))
myTypedArgs
if (state.typedArgs.size != args.length)
state.typedArgs = args.mapconserve(cacheTypedArg(_, typer.typed(_), force))
state.typedArgs
}

def typedArgs: List[Tree] = typedArgs(force = true)
Expand All @@ -306,24 +316,19 @@ object ProtoTypes {
* @pre `arg` has been typed before
*/
def typeOfArg(arg: untpd.Tree)(implicit ctx: Context): Type =
myTypedArg(arg).tpe

private[this] var myTupled: Type = NoType
state.typedArg(arg).tpe

/** The same proto-type but with all arguments combined in a single tuple */
def tupled: FunProto = myTupled match {
def tupled: FunProto = state.tupled match {
case pt: FunProto =>
pt
case _ =>
myTupled = new FunProto(untpd.Tuple(args) :: Nil, resultType, typer)
state.tupled = new FunProto(untpd.Tuple(args) :: Nil, resultType)(typer)
tupled
}

/** Somebody called the `tupled` method of this prototype */
def isTupled: Boolean = myTupled.isInstanceOf[FunProto]

/** If true, the application of this prototype was canceled. */
private[this] var toDrop: Boolean = false
def isTupled: Boolean = state.tupled.isInstanceOf[FunProto]

/** Cancel the application of this prototype. This can happen for a nullary
* application `f()` if `f` refers to a symbol that exists both in parameterless
Expand All @@ -333,10 +338,10 @@ object ProtoTypes {
*/
def markAsDropped() = {
assert(args.isEmpty)
toDrop = true
state.toDrop = true
}

def isDropped: Boolean = toDrop
def isDropped: Boolean = state.toDrop

override def toString = s"FunProto(${args mkString ","} => $resultType)"

Expand All @@ -347,15 +352,20 @@ object ProtoTypes {
ta(ta.foldOver(x, typedArgs.tpes), resultType)

override def deepenProto(implicit ctx: Context) = derivedFunProto(args, resultType.deepenProto, typer)

override def withContext(newCtx: Context) =
if (newCtx `eq` ctx) this
else new FunProto(args, resType)(typer, state)(newCtx)
Copy link
Member

Choose a reason for hiding this comment

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

The idea of cloning FunProtos has promise, but we need to make sure that all clones refer to the same core state.

In what situation are multiple clones of the same FunProto live at the same time, and why is it necessary that they share the same state?

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 noted we had problems with auto-tupling otherwise. The other danger is that we have FP1, infer a type argument, then go to FP2, infer another argument, then decide this is a blocker so fall back to FP1 and try something else afterwards. In this case the computed type argument of FP2 is lost.

}


/** A prototype for expressions that appear in function position
*
* [](args): resultType, where args are known to be typed
*/
class FunProtoTyped(args: List[tpd.Tree], resultType: Type, typer: Typer)(implicit ctx: Context) extends FunProto(args, resultType, typer)(ctx) {
class FunProtoTyped(args: List[tpd.Tree], resultType: Type)(typer: Typer)(implicit ctx: Context) extends FunProto(args, resultType)(typer)(ctx) {
override def typedArgs = args
override def withContext(ctx: Context) = this
}

/** A prototype for implicitly inferred views:
Expand Down Expand Up @@ -392,7 +402,7 @@ object ProtoTypes {
}

class UnapplyFunProto(argType: Type, typer: Typer)(implicit ctx: Context) extends FunProto(
untpd.TypedSplice(dummyTreeOfType(argType))(ctx) :: Nil, WildcardType, typer)
untpd.TypedSplice(dummyTreeOfType(argType))(ctx) :: Nil, WildcardType)(typer)

/** A prototype for expressions [] that are type-parameterized:
*
Expand Down
11 changes: 6 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ class Typer extends Namer
expr1.tpe
case _ =>
val protoArgs = args map (_ withType WildcardType)
val callProto = FunProto(protoArgs, WildcardType, this)
val callProto = FunProto(protoArgs, WildcardType)(this)
val expr1 = typedExpr(expr, callProto)
fnBody = cpy.Apply(fnBody)(untpd.TypedSplice(expr1), args)
expr1.tpe
Expand Down Expand Up @@ -2043,17 +2043,18 @@ class Typer extends Namer
}

def tryApply(implicit ctx: Context) = {
val sel = typedSelect(untpd.Select(untpd.TypedSplice(tree), nme.apply), pt)
val pt1 = pt.withContext(ctx)
val sel = typedSelect(untpd.Select(untpd.TypedSplice(tree), nme.apply), pt1)
sel.pushAttachment(InsertedApply, ())
if (sel.tpe.isError) sel
else try adapt(simplify(sel, pt, locked), pt, locked) finally sel.removeAttachment(InsertedApply)
else try adapt(simplify(sel, pt1, locked), pt1, locked) finally sel.removeAttachment(InsertedApply)
}

def tryImplicit(fallBack: => Tree) =
tryInsertImplicitOnQualifier(tree, pt, locked).getOrElse(fallBack)
tryInsertImplicitOnQualifier(tree, pt.withContext(ctx), locked).getOrElse(fallBack)

pt match {
case pt @ FunProto(Nil, _, _)
case pt @ FunProto(Nil, _)
if tree.symbol.allOverriddenSymbols.exists(_.info.isNullaryMethod) &&
tree.getAttachment(DroppedEmptyArgs).isEmpty =>
tree.putAttachment(DroppedEmptyArgs, ())
Expand Down
3 changes: 3 additions & 0 deletions compiler/test/dotc/pos-from-tasty.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ annot-bootstrap.scala

# ScalaRunTime cannot be unpickled because it is already loaded
repeatedArgs213.scala

# Error printing parent constructors that are blocks
default-super.scala
8 changes: 8 additions & 0 deletions tests/pos/default-super.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class A(x: Int)(y: String = "hi")

object Test {
def f() = 22

class B extends A(f())()

}
11 changes: 11 additions & 0 deletions tests/pos/i4466a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Foo {
def apply(bar: Bar[Int]): Unit = ()
def apply(i: Int): Unit = ()
}

class Bar[X](x: X)

class Main {
val foo = new Foo
foo(new Bar(0))
}
9 changes: 9 additions & 0 deletions tests/pos/i4466b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object Foo {
case class Bar(map: Map[String, String])

object Bar {
def apply(str: String): Bar = ???
}

Bar(Map("A" -> "B"))
}