From 1df849070694756533003c9c331cd04d0c3136e8 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 1 Jul 2014 16:45:33 -0400 Subject: Fix regression around type skolems and if exprs. If we start with: async({ val res = await[S[_$1 with String]](s); if (true) await[Int](0) res }) Typechecking the application (before macro expansion) yields (where the trees are printed in the form `expr{tpe}`): async[S[_$1#5738 with String#137]]({ val res: S[_$1#5490 with String] forSome { type _$1#5490 } = await[S[_$1#5487 with String]]( s{S[_$1#5487 with String]} ){S[_$1#5487 with String]}; if (true) await(0) else () res{S[_$1#5738 with String]} }{S[_$1#5738 with String]}){S[_$1#5738 with String]} Note that the type of the second last line contains a skolemized symbol `_$1#5738` of the existential `_$1#5490`. This is created by this case in `Typer#adapt`: case et @ ExistentialType(_, _) if ((mode & (EXPRmode | LHSmode)) == EXPRmode) => adapt(tree setType et.skolemizeExistential(context.owner, tree), mode, pt, original) Our ANF rewrites part of this code to: val await$1: S[_$1#5487 with String] = await[S[_$1#5487 with String]](awaitable$1); val res: S[_$1#5490 with String] forSome { type _$1 } = await$1; And later, the state machine transformation splits the last line into a blank field and an assignment. Typechecking the `Assign` node led to the an type error. This commit manually attributes the types to the `Assign` node so as to avoid these problem. It also reigns in an overeager rewriting of `If` nodes in the ANF transform, which was due to a bug in the label detection logic introduced in 4fc5463538. Thanks to @gnovark for yet another devilish test case and analysis of the problem with label detection. I worked on a more principled fix on: https://github.com/retronym/async/compare/ticket/79-2?expand=1 in which I try to use `repackExistential` to convert skolemized types to existentials for use as the types of synthetic vals introduced by the ANF transform. This ran into a deeper problem with existential subtyping in the compiler itself though. --- .../scala/async/internal/AsyncTransform.scala | 9 ++-- .../scala/async/internal/TransformUtils.scala | 2 +- .../scala/scala/async/run/ifelse4/IfElse4.scala | 63 ++++++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 src/test/scala/scala/async/run/ifelse4/IfElse4.scala (limited to 'src') diff --git a/src/main/scala/scala/async/internal/AsyncTransform.scala b/src/main/scala/scala/async/internal/AsyncTransform.scala index c8d2234..a24a823 100644 --- a/src/main/scala/scala/async/internal/AsyncTransform.scala +++ b/src/main/scala/scala/async/internal/AsyncTransform.scala @@ -156,9 +156,12 @@ trait AsyncTransform { case ValDef(_, _, _, rhs) if liftedSyms(tree.symbol) => atOwner(currentOwner) { val fieldSym = tree.symbol - val set = Assign(gen.mkAttributedStableRef(fieldSym.owner.thisType, fieldSym), transform(rhs)) - changeOwner(set, tree.symbol, currentOwner) - localTyper.typedPos(tree.pos)(set) + val lhs = atPos(tree.pos) { + gen.mkAttributedStableRef(fieldSym.owner.thisType, fieldSym) + } + val assign = treeCopy.Assign(tree, lhs, transform(rhs)).setType(definitions.UnitTpe) + changeOwner(assign, tree.symbol, currentOwner) + assign } case _: DefTree if liftedSyms(tree.symbol) => EmptyTree diff --git a/src/main/scala/scala/async/internal/TransformUtils.scala b/src/main/scala/scala/async/internal/TransformUtils.scala index f228e1d..bef52f1 100644 --- a/src/main/scala/scala/async/internal/TransformUtils.scala +++ b/src/main/scala/scala/async/internal/TransformUtils.scala @@ -100,7 +100,7 @@ private[async] trait TransformUtils { case ld: LabelDef => ld.symbol }.toSet t.exists { - case rt: RefTree => !(labelDefs contains rt.symbol) + case rt: RefTree => rt.symbol != null && rt.symbol.isLabel && !(labelDefs contains rt.symbol) case _ => false } } diff --git a/src/test/scala/scala/async/run/ifelse4/IfElse4.scala b/src/test/scala/scala/async/run/ifelse4/IfElse4.scala new file mode 100644 index 0000000..b0ecf13 --- /dev/null +++ b/src/test/scala/scala/async/run/ifelse4/IfElse4.scala @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2012-2014 Typesafe Inc. + */ + +package scala.async +package run +package ifelse4 + +import language.{reflectiveCalls, postfixOps} +import scala.concurrent.{Future, ExecutionContext, future, Await} +import scala.concurrent.duration._ +import scala.async.Async.{async, await} +import org.junit.Test + + +class TestIfElse4Class { + + import ExecutionContext.Implicits.global + + class F[A] + class S[A](val id: String) + trait P + + case class K(f: F[_]) + + def result[A](f: F[A]) = async { + new S[A with P]("foo") + } + + def run(k: K) = async { + val res = await(result(k.f)) + // these triggered a crash with mismatched existential skolems + // found : S#10272[_$1#10308 with String#137] where type _$1#10308 + // required: S#10272[_$1#10311 with String#137] forSome { type _$1#10311 } + + // This variation of the crash could be avoided by fixing the over-eager + // generation of states in `If` nodes, which was caused by a bug in label + // detection code. + if(true) { + identity(res) + } + + // This variation remained after the aforementioned fix, however. + // It was fixed by manually typing the `Assign(liftedField, rhs)` AST, + // which is how we avoid these problems through the rest of the ANF transform. + if(true) { + identity(res) + await(result(k.f)) + } + res + } +} + +class IfElse4Spec { + + @Test + def `await result with complex type containing skolem`() { + val o = new TestIfElse4Class + val fut = o.run(new o.K(null)) + val res = Await.result(fut, 2 seconds) + res.id mustBe ("foo") + } +} -- cgit v1.2.3