Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

SI-8285 use correct kind of map for quasiquote positions #3593

Merged
merged 1 commit into from

4 participants

@densh
Owner

Previously mutable.ListMap was used with assumption that it preserved
order of inserted elements (but it doesn't). Surprisingly logic that
assumed ordered elements worked mosly fine on unordered ones. I guess
two bugs can cancel each other out.

review @retronym

@densh densh SI-8285 use correct kind of map for quasiquote positions
Previously mutable.ListMap was used with assumption that it preserved
order of inserted elements (but it doesn't). Surprisingly logic that
assumed ordered elements worked mosly fine on unordered ones. I guess
two bugs can cancel each other out.
51b8e6c
@densh densh added this to the 2.11.0-RC2 milestone
@retronym retronym was assigned by densh
@retronym
Owner

LGTM. You might consider additional refactoring to use Range#contains rather than the < and > checks.

@xeno-by xeno-by merged commit cfac5e9 into from
@adriaanm adriaanm modified the milestone: 2.11.0-RC3, 2.11.0-RC2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 28, 2014
  1. @densh

    SI-8285 use correct kind of map for quasiquote positions

    densh authored
    Previously mutable.ListMap was used with assumption that it preserved
    order of inserted elements (but it doesn't). Surprisingly logic that
    assumed ordered elements worked mosly fine on unordered ones. I guess
    two bugs can cancel each other out.
This page is out of date. Refresh to see the latest.
View
10 src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala
@@ -30,15 +30,15 @@ trait Parsers { self: Quasiquotes =>
def correspondingPosition(offset: Int): Position = {
val posMapList = posMap.toList
- def containsOffset(start: Int, end: Int) = start <= offset && offset <= end
+ def containsOffset(start: Int, end: Int) = start <= offset && offset < end
def fallbackPosition = posMapList match {
case (pos1, (start1, end1)) :: _ if start1 > offset => pos1
- case _ :+ ((pos2, (start2, end2))) if offset > end2 => pos2.withPoint(pos2.point + (end2 - start2))
+ case _ :+ ((pos2, (start2, end2))) if end2 <= offset => pos2.withPoint(pos2.point + (end2 - start2))
}
posMapList.sliding(2).collect {
- case (pos1, (start1, end1)) :: _ if containsOffset(start1, end1) => (pos1, offset - start1)
- case (pos1, (_, end1)) :: (_, (start2, _)) :: _ if containsOffset(end1, start2) => (pos1, end1)
- case _ :: (pos2, (start2, end2)) :: _ if containsOffset(start2, end2) => (pos2, offset - start2)
+ case (pos1, (start1, end1)) :: _ if containsOffset(start1, end1) => (pos1, offset - start1)
+ case (pos1, (start1, end1)) :: (pos2, (start2, _)) :: _ if containsOffset(end1, start2) => (pos1, end1 - start1)
+ case _ :: (pos2, (start2, end2)) :: _ if containsOffset(start2, end2) => (pos2, offset - start2)
}.map { case (pos, offset) =>
pos.withPoint(pos.point + offset)
}.toList.headOption.getOrElse(fallbackPosition)
View
6 src/compiler/scala/tools/reflect/quasiquotes/Placeholders.scala
@@ -17,7 +17,7 @@ trait Placeholders { self: Quasiquotes =>
// Step 1: Transform Scala source with holes into vanilla Scala source
- lazy val posMap = mutable.ListMap[Position, (Int, Int)]()
+ lazy val posMap = mutable.LinkedHashMap[Position, (Int, Int)]()
lazy val code = {
val sb = new StringBuilder()
val sessionSuffix = randomUUID().toString.replace("-", "").substring(0, 8) + "$"
@@ -40,9 +40,7 @@ trait Placeholders { self: Quasiquotes =>
val iargs = method match {
case nme.apply => args
- case nme.unapply =>
- val (dummy @ Ident(nme.SELECTOR_DUMMY)) :: Nil = args
- internal.subpatterns(dummy).get
+ case nme.unapply => internal.subpatterns(args.head).get
case _ => global.abort("unreachable")
}
View
14 test/files/neg/quasiquotes-syntax-error-position.check
@@ -32,4 +32,16 @@ quasiquotes-syntax-error-position.scala:14: error: ')' expected but end of quote
quasiquotes-syntax-error-position.scala:15: error: ':' expected but ')' found.
q"def foo(x)"
^
-11 errors found
+quasiquotes-syntax-error-position.scala:16: error: illegal start of simple expression
+ q"$a(])"
+ ^
+quasiquotes-syntax-error-position.scala:17: error: in XML literal: '>' expected instead of '$'
+ q"foo bar <xml$a>"
+ ^
+quasiquotes-syntax-error-position.scala:19: error: ';' expected but '<:' found.
+ q"val $x: $x <: $x"
+ ^
+quasiquotes-syntax-error-position.scala:20: error: '=' expected but '.' found.
+ q"def f ( $x ) . $x"
+ ^
+15 errors found
View
5 test/files/neg/quasiquotes-syntax-error-position.scala
@@ -13,4 +13,9 @@ object test extends App {
cq"pattern => body ; case pattern2 =>"
pq"$a(bar"
q"def foo(x)"
+ q"$a(])"
+ q"foo bar <xml$a>"
+ val x = q"x"
+ q"val $x: $x <: $x"
+ q"def f ( $x ) . $x"
}
Something went wrong with that request. Please try again.