Skip to content

Commit

Permalink
Fix inconsistent binding in patterns with 10+ holes
Browse files Browse the repository at this point in the history
Previously a map that was storing bindings of fresh hole variables with
their contents (tree & cardinality) used to be a SortedMap which had
issues with inconsistent key ordering:

   "$fresh$prefix$1" < "$fresh$prefix$2"
   ...
   "$fresh$prefix$8" < "$fresh$prefix$9"
   "$fresh$prefix$9" > "$fresh$prefix$10"

This issue is solved by using a LinkedHashMap instead (keys are inserted
in the proper order.)
  • Loading branch information
densh committed Feb 6, 2014
1 parent fc15cfc commit 8994da9
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 21 deletions.
37 changes: 19 additions & 18 deletions src/compiler/scala/tools/reflect/quasiquotes/Placeholders.scala
Expand Up @@ -17,7 +17,6 @@ trait Placeholders { self: Quasiquotes =>

// Step 1: Transform Scala source with holes into vanilla Scala source

lazy val holeMap = new HoleMap()
lazy val posMap = mutable.ListMap[Position, (Int, Int)]()
lazy val code = {
val sb = new StringBuilder()
Expand Down Expand Up @@ -58,25 +57,27 @@ trait Placeholders { self: Quasiquotes =>
sb.toString
}

class HoleMap {
private var underlying = immutable.SortedMap[String, Hole]()
private val accessed = mutable.Set[String]()
object holeMap {
private val underlying = mutable.LinkedHashMap.empty[String, Hole]
private val accessed = mutable.Set.empty[String]
def unused: Set[Name] = (underlying.keys.toSet -- accessed).map(TermName(_))
def contains(key: Name) = underlying.contains(key.toString)
def apply(key: Name) = {
val s = key.toString
accessed += s
underlying(s)
}
def update(key: Name, hole: Hole) = {
def contains(key: Name): Boolean = underlying.contains(key.toString)
def apply(key: Name): Hole = {
val skey = key.toString
val value = underlying(skey)
accessed += skey
value
}
def update(key: Name, hole: Hole) =
underlying += key.toString -> hole
def get(key: Name): Option[Hole] = {
val skey = key.toString
underlying.get(skey).map { v =>
accessed += skey
v
}
}
def get(key: Name) = {
val s = key.toString
accessed += s
underlying.get(s)
}
def toList = underlying.toList
def keysIterator: Iterator[TermName] = underlying.keysIterator.map(TermName(_))
}

// Step 2: Transform vanilla Scala AST into an AST with holes
Expand Down Expand Up @@ -179,4 +180,4 @@ trait Placeholders { self: Quasiquotes =>
case _ => None
}
}
}
}
6 changes: 3 additions & 3 deletions src/compiler/scala/tools/reflect/quasiquotes/Reifiers.scala
Expand Up @@ -29,7 +29,7 @@ trait Reifiers { self: Quasiquotes =>
/** Map that stores freshly generated names linked to the corresponding names in the reified tree.
* This information is used to reify names created by calls to freshTermName and freshTypeName.
*/
var nameMap = collection.mutable.HashMap.empty[Name, Set[TermName]].withDefault { _ => Set() }
val nameMap = collection.mutable.HashMap.empty[Name, Set[TermName]].withDefault { _ => Set() }

/** Wraps expressions into:
* a block which starts with a sequence of vals that correspond
Expand Down Expand Up @@ -71,7 +71,7 @@ trait Reifiers { self: Quasiquotes =>
// q"..$freshdefs; $tree"
SyntacticBlock(freshdefs :+ tree)
} else {
val freevars = holeMap.toList.map { case (name, _) => Ident(name) }
val freevars = holeMap.keysIterator.map(Ident(_)).toList
val isVarPattern = tree match { case Bind(name, Ident(nme.WILDCARD)) => true case _ => false }
val cases =
if(isVarPattern) {
Expand Down Expand Up @@ -162,7 +162,7 @@ trait Reifiers { self: Quasiquotes =>
reifyBuildCall(nme.SyntacticNew, earlyDefs, parents, selfdef, body)
case SyntacticDefDef(mods, name, tparams, build.ImplicitParams(vparamss, implparams), tpt, rhs) =>
if (implparams.nonEmpty)
mirrorBuildCall(nme.SyntacticDefDef, reify(mods), reify(name), reify(tparams),
mirrorBuildCall(nme.SyntacticDefDef, reify(mods), reify(name), reify(tparams),
reifyBuildCall(nme.ImplicitParams, vparamss, implparams), reify(tpt), reify(rhs))
else
reifyBuildCall(nme.SyntacticDefDef, mods, name, tparams, vparamss, tpt, rhs)
Expand Down
5 changes: 5 additions & 0 deletions test/files/scalacheck/quasiquotes/TermConstructionProps.scala
Expand Up @@ -291,4 +291,9 @@ object TermConstructionProps extends QuasiquoteProperties("term construction") {
val stats2 = List.empty[Tree]
assert(q"{ ..$stats2 }"q"")
}

property("consistent variable order") = test {
val q"$a = $b = $c = $d = $e = $f = $g = $h = $k = $l" = q"a = b = c = d = e = f = g = h = k = l"
assert(a ≈ q"a" && b ≈ q"b" && c ≈ q"c" && d ≈ q"d" && e ≈ q"e" && g ≈ q"g" && h ≈ q"h" && k ≈ q"k" && l ≈ q"l")
}
}

0 comments on commit 8994da9

Please sign in to comment.