From f809b066ba220568921f611e652301590abb0b3a Mon Sep 17 00:00:00 2001 From: Tomas Janousek Date: Tue, 4 Aug 2015 16:12:50 +0200 Subject: [PATCH 1/2] Fix packrat caching with PagedSeqReader This fixes #45. The problem was that whenever PagedSeqReader is constructed, source is assigned seq (although lazily) but since PagedSeq is not a subclass of java.lang.CharSequence, an implicit conversion takes place via Predef#SeqCharSequence, creating a new object. The problem is that this happens every time PagedSeqReader#rest or drop is called, breaking packrat caching entirely. --- .../scala/util/parsing/input/PagedSeqReader.scala | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/scala/scala/util/parsing/input/PagedSeqReader.scala b/src/main/scala/scala/util/parsing/input/PagedSeqReader.scala index 500c80c7..51e66943 100644 --- a/src/main/scala/scala/util/parsing/input/PagedSeqReader.scala +++ b/src/main/scala/scala/util/parsing/input/PagedSeqReader.scala @@ -30,10 +30,10 @@ object PagedSeqReader { * @author Martin Odersky */ class PagedSeqReader(seq: PagedSeq[Char], - override val offset: Int) extends Reader[Char] { + override val offset: Int) extends Reader[Char] { outer => import PagedSeqReader._ - override lazy val source: java.lang.CharSequence = seq + override val source: java.lang.CharSequence = seq /** Construct a `PagedSeqReader` with its first element at * `source(0)` and position `(1,1)`. @@ -51,7 +51,9 @@ class PagedSeqReader(seq: PagedSeq[Char], * otherwise, it's a `PagedSeqReader` containing the rest of input. */ def rest: PagedSeqReader = - if (seq.isDefinedAt(offset)) new PagedSeqReader(seq, offset + 1) + if (seq.isDefinedAt(offset)) new PagedSeqReader(seq, offset + 1) { + override val source: java.lang.CharSequence = outer.source + } else this /** The position of the first element in the reader. @@ -67,5 +69,7 @@ class PagedSeqReader(seq: PagedSeq[Char], * `n` elements. */ override def drop(n: Int): PagedSeqReader = - new PagedSeqReader(seq, offset + n) + new PagedSeqReader(seq, offset + n) { + override val source: java.lang.CharSequence = outer.source + } } From 0345e49849da4fa33df8c87d20f6920b0f1df0f0 Mon Sep 17 00:00:00 2001 From: Tomas Janousek Date: Sat, 8 Aug 2015 11:54:18 +0200 Subject: [PATCH 2/2] Add test for #45 --- .../scala/util/parsing/combinator/gh45.scala | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 src/test/scala/scala/util/parsing/combinator/gh45.scala diff --git a/src/test/scala/scala/util/parsing/combinator/gh45.scala b/src/test/scala/scala/util/parsing/combinator/gh45.scala new file mode 100644 index 00000000..e184719a --- /dev/null +++ b/src/test/scala/scala/util/parsing/combinator/gh45.scala @@ -0,0 +1,46 @@ +package scala.util.parsing.combinator + +import scala.util.parsing.input._ +import scala.collection.immutable.PagedSeq + +import org.junit.Test +import org.junit.Assert.assertTrue + +import scala.util.parsing.combinator.syntactical.StandardTokenParsers + +class gh45 { + + @Test + def test4: Unit = { + def check(rd: Reader[Char]): Unit = { + val g = new grammar + val p = g.phrase(g.script) + val parseResult = p(new g.lexical.Scanner(rd)) + assertTrue(parseResult.isInstanceOf[g.Success[_]]) + } + + val str = "x once y" + check(new CharSequenceReader(str)) + /* Note that this only tests PagedSeq.rest since neither + * PackratReader nor lexical.Scanner override/use the drop method. + */ + check(new PagedSeqReader(PagedSeq.fromStrings(List(str)))) + } + +} + +private final class grammar extends StandardTokenParsers with PackratParsers { + lexical.reserved ++= List("x", "y", "z", "once") + + var onceCnt: Int = 0 + lazy val once: PackratParser[String] = memo("once") ^? { + case s if onceCnt == 0 => + onceCnt += 1 + s + } + + lazy val script: PackratParser[Any] = + ( "x" ~ once ~ "z" + | "x" ~ once ~ "y" + ) +}