Skip to content

Commit

Permalink
Determistic output from the async macro
Browse files Browse the repository at this point in the history
Eliminates the failures reported by the new `testDeterminism` task:

Before:
```
% sbt testDeterminism

[error] java.lang.AssertionError: /Users/jz/code/scala-async/target/scala-2.12/test-classes-baseline/scala/async/neg/LocalClasses0Spec$stateMachine$async$2.class is not equal to /Users/jz/code/scala-async/target/scala-2.12/test-classes/scala/async/neg/LocalClasses0Spec$stateMachine$async$2.class
[error] Use 'last' for the full log.

$ jardiff /Users/jz/code/scala-async/target/scala-2.12/test-classes-baseline /Users/jz/code/scala-async/target/scala-2.12/test-classes
diff --git a/scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.class.asm b/scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.class.asm
index f6f7d26..585e1b5 100644
--- a/scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.class.asm
+++ b/scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.class.asm
@@ -256,10 +256,10 @@
     PUTFIELD scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.state$async : I
     ALOAD 0
     ACONST_NULL
-    PUTFIELD scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.await$async$0 : Lscala/async/run/ifelse4/TestIfElse4Class$S;
+    PUTFIELD scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.await$async$1 : Lscala/async/run/ifelse4/TestIfElse4Class$S;
     ALOAD 0
     ACONST_NULL
-    PUTFIELD scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.await$async$1 : Lscala/async/run/ifelse4/TestIfElse4Class$S;
+    PUTFIELD scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.await$async$0 : Lscala/async/run/ifelse4/TestIfElse4Class$S;
     ALOAD 0
     GETSTATIC scala/runtime/BoxedUnit.UNIT : Lscala/runtime/BoxedUnit;
     PUTFIELD scala/async/run/ifelse4/TestIfElse4Class$stateMachine$async$1.match$async$1 : Ljava/lang/Object;
diff --git a/scala/async/run/match0/MatchSpec$stateMachine$async$5.class.asm b/scala/async/run/match0/MatchSpec$stateMachine$async$5.class.asm
index caa05ea..fac9cd9 100644
--- a/scala/async/run/match0/MatchSpec$stateMachine$async$5.class.asm
+++ b/scala/async/run/match0/MatchSpec$stateMachine$async$5.class.asm
@@ -88,7 +88,7 @@
    L3
     ALOAD 0
     ICONST_0
-    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0$async$1 : I
+    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0 : I
     LDC ""
     INVOKEVIRTUAL java/lang/String.isEmpty ()Z
     IFEQ L17
@@ -123,7 +123,7 @@
     ALOAD 0
     ALOAD 0
     GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.await$async$0 : I
-    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0$async$1 : I
+    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0 : I
     GOTO L1
    L7
     ALOAD 0
@@ -135,7 +135,7 @@
     ATHROW
    L8
     ALOAD 0
-    GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0$async$1 : I
+    GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0 : I
     ISTORE 5
     ALOAD 0
     BIPUSH 6
@@ -151,7 +151,7 @@
    L10
     ALOAD 0
     ICONST_0
-    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0 : I
+    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0$async$1 : I
     ALOAD 0
     GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.await$async$1 : I
     ISTORE 6
@@ -216,7 +216,7 @@
     GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.y$async$1 : I
     ILOAD 10
     IMUL
-    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0 : I
+    PUTFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0$async$1 : I
     GOTO L1
    L15
     ALOAD 0
@@ -224,7 +224,7 @@
     NEW scala/util/Success
     DUP
     ALOAD 0
-    GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0 : I
+    GETFIELD scala/async/run/match0/MatchSpec$stateMachine$async$5.match$async$0$async$1 : I
     INVOKESTATIC scala/runtime/BoxesRunTime.boxToInteger (I)Ljava/lang/Integer;
     INVOKESPECIAL scala/util/Success.<init> (Ljava/lang/Object;)V
     INVOKEVIRTUAL scala/util/Success.get ()Ljava/lang/Object;
diff --git a/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.class.asm b/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.class.asm
index 642ac1b..d637739 100644
--- a/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.class.asm
+++ b/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.class.asm
@@ -219,6 +219,9 @@
     ICONST_5
     PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.state$async : I
     ALOAD 0
+    ACONST_NULL
+    PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.await$async$0 : Lscala/None$;
+    ALOAD 0
     NEW scala/async/run/toughtype/ParamWrapper
     DUP
     ACONST_NULL
@@ -234,9 +237,6 @@
     CHECKCAST java/lang/String
    L23
     PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.valueHolder$async$1 : Ljava/lang/String;
-    ALOAD 0
-    ACONST_NULL
-    PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.await$async$0 : Lscala/None$;
     ALOAD 0
     GETSTATIC scala/None$.MODULE$ : Lscala/None$;
     PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$17.match$async$0 : Lscala/None$;
diff --git a/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.class.asm b/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.class.asm
index 5c15112..0b81dba 100644
--- a/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.class.asm
+++ b/scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.class.asm
@@ -216,9 +216,6 @@
     ICONST_5
     PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.state$async : I
     ALOAD 0
-    ACONST_NULL
-    PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.await$async$0 : Lscala/None$;
-    ALOAD 0
     NEW scala/async/run/toughtype/PrivateWrapper
     DUP
     ACONST_NULL
@@ -234,6 +231,9 @@
    L23
     PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.valueHolder$async$1 : Ljava/lang/String;
     ALOAD 0
+    ACONST_NULL
+    PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.await$async$0 : Lscala/None$;
+    ALOAD 0
     GETSTATIC scala/None$.MODULE$ : Lscala/None$;
     PUTFIELD scala/async/run/toughtype/ToughTypeSpec$stateMachine$async$18.match$async$0 : Lscala/None$;
     GOTO L1
```

After:

```
$ for i in {1..10}; do sbt testDeterminism || break; done
...
<NO ERRORS>
```
  • Loading branch information
retronym committed Nov 15, 2018
1 parent 6ecaf0f commit 2c4ac2f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 26 deletions.
35 changes: 18 additions & 17 deletions src/main/scala/scala/async/internal/Lifter.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package scala.async.internal

import scala.collection.mutable

trait Lifter {
self: AsyncMacro =>
import c.universe._
Expand Down Expand Up @@ -37,7 +39,7 @@ trait Lifter {
}


val defs: Map[Tree, Int] = {
val defs: mutable.LinkedHashMap[Tree, Int] = {
/** Collect the DefTrees directly enclosed within `t` that have the same owner */
def collectDirectlyEnclosedDefs(t: Tree): List[DefTree] = t match {
case ld: LabelDef => Nil
Expand All @@ -48,33 +50,33 @@ trait Lifter {
companionship.record(childDefs)
childDefs
}
asyncStates.flatMap {
mutable.LinkedHashMap(asyncStates.flatMap {
asyncState =>
val defs = collectDirectlyEnclosedDefs(Block(asyncState.allStats: _*))
defs.map((_, asyncState.state))
}.toMap
}: _*)
}

// In which block are these symbols defined?
val symToDefiningState: Map[Symbol, Int] = defs.map {
val symToDefiningState: mutable.LinkedHashMap[Symbol, Int] = defs.map {
case (k, v) => (k.symbol, v)
}

// The definitions trees
val symToTree: Map[Symbol, Tree] = defs.map {
val symToTree: mutable.LinkedHashMap[Symbol, Tree] = defs.map {
case (k, v) => (k.symbol, k)
}

// The direct references of each definition tree
val defSymToReferenced: Map[Symbol, List[Symbol]] = defs.keys.map {
case tree => (tree.symbol, tree.collect {
val defSymToReferenced: mutable.LinkedHashMap[Symbol, List[Symbol]] = defs.map {
case (tree, _) => (tree.symbol, tree.collect {
case rt: RefTree if symToDefiningState.contains(rt.symbol) => rt.symbol
})
}.toMap
}

// The direct references of each block, excluding references of `DefTree`-s which
// are already accounted for.
val stateIdToDirectlyReferenced: Map[Int, List[Symbol]] = {
val stateIdToDirectlyReferenced: mutable.LinkedHashMap[Int, List[Symbol]] = {
val refs: List[(Int, Symbol)] = asyncStates.flatMap(
asyncState => asyncState.stats.filterNot(t => t.isDef && !isLabel(t.symbol)).flatMap(_.collect {
case rt: RefTree
Expand All @@ -84,8 +86,8 @@ trait Lifter {
toMultiMap(refs)
}

def liftableSyms: Set[Symbol] = {
val liftableMutableSet = collection.mutable.Set[Symbol]()
def liftableSyms: mutable.LinkedHashSet[Symbol] = {
val liftableMutableSet = mutable.LinkedHashSet[Symbol]()
def markForLift(sym: Symbol): Unit = {
if (!liftableMutableSet(sym)) {
liftableMutableSet += sym
Expand All @@ -97,19 +99,19 @@ trait Lifter {
}
}
// Start things with DefTrees directly referenced from statements from other states...
val liftableStatementRefs: List[Symbol] = stateIdToDirectlyReferenced.toList.flatMap {
val liftableStatementRefs: List[Symbol] = stateIdToDirectlyReferenced.iterator.flatMap {
case (i, syms) => syms.filter(sym => symToDefiningState(sym) != i)
}
}.toList
// .. and likewise for DefTrees directly referenced by other DefTrees from other states
val liftableRefsOfDefTrees = defSymToReferenced.toList.flatMap {
case (referee, referents) => referents.filter(sym => symToDefiningState(sym) != symToDefiningState(referee))
}
// Mark these for lifting, which will follow transitive references.
(liftableStatementRefs ++ liftableRefsOfDefTrees).foreach(markForLift)
liftableMutableSet.toSet
liftableMutableSet
}

val lifted = liftableSyms.map(symToTree).toList.map {
liftableSyms.iterator.map(symToTree).map {
t =>
val sym = t.symbol
val treeLifted = t match {
Expand Down Expand Up @@ -147,7 +149,6 @@ trait Lifter {
treeCopy.TypeDef(td, Modifiers(sym.flags), sym.name, tparams, rhs)
}
atPos(t.pos)(treeLifted)
}
lifted
}.toList
}
}
16 changes: 9 additions & 7 deletions src/main/scala/scala/async/internal/LiveVariables.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package scala.async.internal

import scala.collection.mutable

import java.util
import java.util.function.{IntConsumer, IntPredicate}

Expand All @@ -19,12 +21,12 @@ trait LiveVariables {
* @return a map mapping a state to the fields that should be nulled out
* upon resuming that state
*/
def fieldsToNullOut(asyncStates: List[AsyncState], liftables: List[Tree]): Map[Int, List[Tree]] = {
def fieldsToNullOut(asyncStates: List[AsyncState], liftables: List[Tree]): mutable.LinkedHashMap[Int, List[Tree]] = {
// live variables analysis:
// the result map indicates in which states a given field should be nulled out
val liveVarsMap: Map[Tree, StateSet] = liveVars(asyncStates, liftables)
val liveVarsMap: mutable.LinkedHashMap[Tree, StateSet] = liveVars(asyncStates, liftables)

var assignsOf = Map[Int, List[Tree]]()
var assignsOf = mutable.LinkedHashMap[Int, List[Tree]]()

for ((fld, where) <- liveVarsMap) {
where.foreach { new IntConsumer { def accept(state: Int): Unit = {
Expand Down Expand Up @@ -54,7 +56,7 @@ trait LiveVariables {
* @param liftables the lifted fields
* @return a map which indicates for a given field (the key) the states in which it should be nulled out
*/
def liveVars(asyncStates: List[AsyncState], liftables: List[Tree]): Map[Tree, StateSet] = {
def liveVars(asyncStates: List[AsyncState], liftables: List[Tree]): mutable.LinkedHashMap[Tree, StateSet] = {
val liftedSyms: Set[Symbol] = // include only vars
liftables.iterator.filter {
case ValDef(mods, _, _, _) => mods.hasFlag(MUTABLE)
Expand Down Expand Up @@ -262,15 +264,15 @@ trait LiveVariables {
result
}

val lastUsages: Map[Tree, StateSet] =
liftables.iterator.map(fld => fld -> lastUsagesOf(fld, finalState)).toMap
val lastUsages: mutable.LinkedHashMap[Tree, StateSet] =
mutable.LinkedHashMap(liftables.map(fld => fld -> lastUsagesOf(fld, finalState)): _*)

if(AsyncUtils.verbose) {
for ((fld, lastStates) <- lastUsages)
AsyncUtils.vprintln(s"field ${fld.symbol.name} is last used in states ${lastStates.iterator.mkString(", ")}")
}

val nullOutAt: Map[Tree, StateSet] =
val nullOutAt: mutable.LinkedHashMap[Tree, StateSet] =
for ((fld, lastStates) <- lastUsages) yield {
var result = new StateSet
lastStates.foreach(new IntConsumer { def accept(s: Int): Unit = {
Expand Down
13 changes: 11 additions & 2 deletions src/main/scala/scala/async/internal/TransformUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package scala.async.internal
import scala.reflect.macros.Context
import reflect.ClassTag
import scala.collection.immutable.ListMap
import scala.collection.mutable
import scala.collection.mutable.ListBuffer

/**
* Utilities used in both `ExprBuilder` and `AnfTransform`.
Expand Down Expand Up @@ -303,8 +305,15 @@ private[async] trait TransformUtils {
})
}

def toMultiMap[A, B](as: Iterable[(A, B)]): Map[A, List[B]] =
as.toList.groupBy(_._1).mapValues(_.map(_._2).toList).toMap
def toMultiMap[A, B](abs: Iterable[(A, B)]): mutable.LinkedHashMap[A, List[B]] = {
// LinkedHashMap for stable order of results.
val result = new mutable.LinkedHashMap[A, ListBuffer[B]]()
for ((a, b) <- abs) {
val buffer = result.getOrElseUpdate(a, new ListBuffer[B])
buffer += b
}
result.map { case (a, b) => (a, b.toList) }
}

// Attributed version of `TreeGen#mkCastPreservingAnnotations`
def mkAttributedCastPreservingAnnotations(tree: Tree, tp: Type): Tree = {
Expand Down

0 comments on commit 2c4ac2f

Please sign in to comment.