From ea427b93144bfcdbe5da102b9110ad352a06e96d Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Thu, 14 Oct 2021 21:29:06 -0400 Subject: [PATCH 1/6] Defer creation of expectations --- .../src/main/scala/cats/parse/Parser.scala | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala index 40363278..8a18a5f8 100644 --- a/core/shared/src/main/scala/cats/parse/Parser.scala +++ b/core/shared/src/main/scala/cats/parse/Parser.scala @@ -68,7 +68,7 @@ sealed abstract class Parser0[+A] { self: Product => val offset = state.offset if (err eq null) Right((str.substring(offset), result)) else - Left(Parser.Error(offset, Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.toList)))) + Left(Parser.Error(offset, Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)))) } /** Attempt to parse all of the input `str` into an `A` value. @@ -92,7 +92,7 @@ sealed abstract class Parser0[+A] { self: Product => ) ) } else - Left(Parser.Error(offset, Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.toList)))) + Left(Parser.Error(offset, Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)))) } /** Convert epsilon failures into None values. @@ -1773,7 +1773,7 @@ object Parser { */ protected[parse] final class State(val str: String) { var offset: Int = 0 - var error: Chain[Expectation] = null + var error: Eval[Chain[Expectation]] = null var capture: Boolean = true } @@ -2033,7 +2033,7 @@ object Parser { state.offset = end res } else { - state.error = Chain.one(Expectation.Length(offset, len, state.str.length - offset)) + state.error = Eval.later(Chain.one(Expectation.Length(offset, len, state.str.length - offset))) null } } @@ -2079,8 +2079,9 @@ object Parser { case object StartParser extends Parser0[Unit] { override def parseMut(state: State): Unit = { - if (state.offset != 0) { - state.error = Chain.one(Expectation.StartOfString(state.offset)) + val offset = state.offset + if (offset != 0) { + state.error = Eval.later(Chain.one(Expectation.StartOfString(offset))) } () } @@ -2088,8 +2089,9 @@ object Parser { case object EndParser extends Parser0[Unit] { override def parseMut(state: State): Unit = { - if (state.offset != state.str.length) { - state.error = Chain.one(Expectation.EndOfString(state.offset, state.str.length)) + val offset = state.offset + if (offset != state.str.length) { + state.error = Eval.later(Chain.one(Expectation.EndOfString(offset, state.str.length))) } () } @@ -2128,7 +2130,7 @@ object Parser { state.offset += message.length () } else { - state.error = Chain.one(Expectation.OneOfStr(offset, message :: Nil)) + state.error = Eval.later(Chain.one(Expectation.OneOfStr(offset, message :: Nil))) () } } @@ -2144,7 +2146,7 @@ object Parser { state.offset += message.length () } else { - state.error = Chain.one(Expectation.OneOfStr(offset, message :: Nil)) + state.error = Eval.later(Chain.one(Expectation.OneOfStr(offset, message :: Nil))) () } } @@ -2152,21 +2154,23 @@ object Parser { case class Fail[A]() extends Parser[A] { override def parseMut(state: State): A = { - state.error = Chain.one(Expectation.Fail(state.offset)); + val offset = state.offset + state.error = Eval.later(Chain.one(Expectation.Fail(offset))) null.asInstanceOf[A] } } case class FailWith[A](message: String) extends Parser[A] { override def parseMut(state: State): A = { - state.error = Chain.one(Expectation.FailWith(state.offset, message)); + val offset = state.offset + state.error = Eval.later(Chain.one(Expectation.FailWith(offset, message))) null.asInstanceOf[A] } } final def oneOf[A](all: Array[Parser0[A]], state: State): A = { val offset = state.offset - var errs: Chain[Expectation] = Chain.nil + var errs: Eval[Chain[Expectation]] = Eval.later(Chain.nil) var idx = 0 while (idx < all.length) { val thisParser = all(idx) @@ -2180,7 +2184,7 @@ object Parser { // we failed to parse, but didn't consume input // is unchanged we continue // else we stop - errs = errs ++ err + errs = errs.map(_ ++ err.value) state.error = null idx = idx + 1 } @@ -2221,7 +2225,7 @@ object Parser { } } if (lastMatch < 0) { - state.error = Chain.one(Expectation.OneOfStr(startOffset, all.toList)) + state.error = Eval.later(Chain.one(Expectation.OneOfStr(startOffset, all.toList))) state.offset = startOffset } else { state.offset = lastMatch @@ -2645,7 +2649,7 @@ object Parser { state.offset += 1 char } else { - state.error = Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue)) + state.error = Eval.later(Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) '\u0000' } } @@ -2656,15 +2660,12 @@ object Parser { override def toString = s"CharIn($min, bitSet = ..., $ranges)" - def makeError(offset: Int): Chain[Expectation] = { - var result = Chain.empty[Expectation] - var aux = ranges.toList - while (aux.nonEmpty) { - val (s, e) = aux.head - result = result :+ Expectation.InRange(offset, s, e) - aux = aux.tail - } - result + def makeError(offset: Int): Eval[Chain[Expectation]] = { + Eval.later(Chain.fromSeq( + ranges.toList.map { case (s, e) => + Expectation.InRange(offset, s, e) + } + )) } override def parseMut(state: State): Char = { @@ -2703,7 +2704,7 @@ object Parser { val matchedStr = state.str.substring(offset, state.offset) // we don't reset the offset, so if the underlying parser // advanced it will fail in a OneOf - state.error = Chain.one(Expectation.ExpectedFailureAt(offset, matchedStr)) + state.error = Eval.later(Chain.one(Expectation.ExpectedFailureAt(offset, matchedStr))) } state.offset = offset @@ -2732,7 +2733,7 @@ object Parser { override def parseMut(state: State): A = { val a = under.parseMut(state) if (state.error ne null) { - state.error = state.error.map(Expectation.WithContext(context, _)) + state.error = state.error.map(_.map(Expectation.WithContext(context, _))) } a } @@ -2742,7 +2743,7 @@ object Parser { override def parseMut(state: State): A = { val a = under.parseMut(state) if (state.error ne null) { - state.error = state.error.map(Expectation.WithContext(context, _)) + state.error = state.error.map(_.map(Expectation.WithContext(context, _))) } a } From 7de43723a37f3faf7fda4e3644311243a054c04d Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Thu, 14 Oct 2021 21:53:39 -0400 Subject: [PATCH 2/6] scalafmt --- .../src/main/scala/cats/parse/Parser.scala | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala index 8a18a5f8..9d978fe1 100644 --- a/core/shared/src/main/scala/cats/parse/Parser.scala +++ b/core/shared/src/main/scala/cats/parse/Parser.scala @@ -68,7 +68,12 @@ sealed abstract class Parser0[+A] { self: Product => val offset = state.offset if (err eq null) Right((str.substring(offset), result)) else - Left(Parser.Error(offset, Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)))) + Left( + Parser.Error( + offset, + Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)) + ) + ) } /** Attempt to parse all of the input `str` into an `A` value. @@ -92,7 +97,12 @@ sealed abstract class Parser0[+A] { self: Product => ) ) } else - Left(Parser.Error(offset, Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)))) + Left( + Parser.Error( + offset, + Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)) + ) + ) } /** Convert epsilon failures into None values. @@ -2033,7 +2043,8 @@ object Parser { state.offset = end res } else { - state.error = Eval.later(Chain.one(Expectation.Length(offset, len, state.str.length - offset))) + state.error = + Eval.later(Chain.one(Expectation.Length(offset, len, state.str.length - offset))) null } } @@ -2649,7 +2660,8 @@ object Parser { state.offset += 1 char } else { - state.error = Eval.later(Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) + state.error = + Eval.later(Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) '\u0000' } } @@ -2661,11 +2673,13 @@ object Parser { override def toString = s"CharIn($min, bitSet = ..., $ranges)" def makeError(offset: Int): Eval[Chain[Expectation]] = { - Eval.later(Chain.fromSeq( - ranges.toList.map { case (s, e) => - Expectation.InRange(offset, s, e) - } - )) + Eval.later( + Chain.fromSeq( + ranges.toList.map { case (s, e) => + Expectation.InRange(offset, s, e) + } + ) + ) } override def parseMut(state: State): Char = { From dedd158cf1cbf0c7415278f9ef18548675bd322d Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Thu, 14 Oct 2021 22:16:46 -0400 Subject: [PATCH 3/6] s/Eval/thunks/g --- .../src/main/scala/cats/parse/Parser.scala | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala index 9d978fe1..70242db3 100644 --- a/core/shared/src/main/scala/cats/parse/Parser.scala +++ b/core/shared/src/main/scala/cats/parse/Parser.scala @@ -71,7 +71,7 @@ sealed abstract class Parser0[+A] { self: Product => Left( Parser.Error( offset, - Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)) + Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err().toList)) ) ) } @@ -100,7 +100,7 @@ sealed abstract class Parser0[+A] { self: Product => Left( Parser.Error( offset, - Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)) + Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err().toList)) ) ) } @@ -1783,7 +1783,7 @@ object Parser { */ protected[parse] final class State(val str: String) { var offset: Int = 0 - var error: Eval[Chain[Expectation]] = null + var error: () => Chain[Expectation] = null var capture: Boolean = true } @@ -2043,8 +2043,7 @@ object Parser { state.offset = end res } else { - state.error = - Eval.later(Chain.one(Expectation.Length(offset, len, state.str.length - offset))) + state.error = () => Chain.one(Expectation.Length(offset, len, state.str.length - offset)) null } } @@ -2092,7 +2091,7 @@ object Parser { override def parseMut(state: State): Unit = { val offset = state.offset if (offset != 0) { - state.error = Eval.later(Chain.one(Expectation.StartOfString(offset))) + state.error = () => (Chain.one(Expectation.StartOfString(offset))) } () } @@ -2102,7 +2101,7 @@ object Parser { override def parseMut(state: State): Unit = { val offset = state.offset if (offset != state.str.length) { - state.error = Eval.later(Chain.one(Expectation.EndOfString(offset, state.str.length))) + state.error = () => (Chain.one(Expectation.EndOfString(offset, state.str.length))) } () } @@ -2141,7 +2140,7 @@ object Parser { state.offset += message.length () } else { - state.error = Eval.later(Chain.one(Expectation.OneOfStr(offset, message :: Nil))) + state.error = () => (Chain.one(Expectation.OneOfStr(offset, message :: Nil))) () } } @@ -2157,7 +2156,7 @@ object Parser { state.offset += message.length () } else { - state.error = Eval.later(Chain.one(Expectation.OneOfStr(offset, message :: Nil))) + state.error = () => (Chain.one(Expectation.OneOfStr(offset, message :: Nil))) () } } @@ -2166,7 +2165,7 @@ object Parser { case class Fail[A]() extends Parser[A] { override def parseMut(state: State): A = { val offset = state.offset - state.error = Eval.later(Chain.one(Expectation.Fail(offset))) + state.error = () => (Chain.one(Expectation.Fail(offset))) null.asInstanceOf[A] } } @@ -2174,14 +2173,14 @@ object Parser { case class FailWith[A](message: String) extends Parser[A] { override def parseMut(state: State): A = { val offset = state.offset - state.error = Eval.later(Chain.one(Expectation.FailWith(offset, message))) + state.error = () => (Chain.one(Expectation.FailWith(offset, message))) null.asInstanceOf[A] } } final def oneOf[A](all: Array[Parser0[A]], state: State): A = { val offset = state.offset - var errs: Eval[Chain[Expectation]] = Eval.later(Chain.nil) + var errs: () => Chain[Expectation] = () => Chain.nil var idx = 0 while (idx < all.length) { val thisParser = all(idx) @@ -2195,7 +2194,7 @@ object Parser { // we failed to parse, but didn't consume input // is unchanged we continue // else we stop - errs = errs.map(_ ++ err.value) + errs = () => errs() ++ err() state.error = null idx = idx + 1 } @@ -2236,7 +2235,7 @@ object Parser { } } if (lastMatch < 0) { - state.error = Eval.later(Chain.one(Expectation.OneOfStr(startOffset, all.toList))) + state.error = () => (Chain.one(Expectation.OneOfStr(startOffset, all.toList))) state.offset = startOffset } else { state.offset = lastMatch @@ -2660,8 +2659,7 @@ object Parser { state.offset += 1 char } else { - state.error = - Eval.later(Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) + state.error = () => (Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) '\u0000' } } @@ -2672,8 +2670,8 @@ object Parser { override def toString = s"CharIn($min, bitSet = ..., $ranges)" - def makeError(offset: Int): Eval[Chain[Expectation]] = { - Eval.later( + def makeError(offset: Int): () => Chain[Expectation] = { () => + ( Chain.fromSeq( ranges.toList.map { case (s, e) => Expectation.InRange(offset, s, e) @@ -2718,7 +2716,7 @@ object Parser { val matchedStr = state.str.substring(offset, state.offset) // we don't reset the offset, so if the underlying parser // advanced it will fail in a OneOf - state.error = Eval.later(Chain.one(Expectation.ExpectedFailureAt(offset, matchedStr))) + state.error = () => (Chain.one(Expectation.ExpectedFailureAt(offset, matchedStr))) } state.offset = offset From c52a6bbc4a64db2868279bbe3626e5612f950272 Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Thu, 14 Oct 2021 22:50:53 -0400 Subject: [PATCH 4/6] Revert "s/Eval/thunks/g" -- not stack safe This reverts commit dedd158cf1cbf0c7415278f9ef18548675bd322d. --- .../src/main/scala/cats/parse/Parser.scala | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala index 70242db3..9d978fe1 100644 --- a/core/shared/src/main/scala/cats/parse/Parser.scala +++ b/core/shared/src/main/scala/cats/parse/Parser.scala @@ -71,7 +71,7 @@ sealed abstract class Parser0[+A] { self: Product => Left( Parser.Error( offset, - Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err().toList)) + Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)) ) ) } @@ -100,7 +100,7 @@ sealed abstract class Parser0[+A] { self: Product => Left( Parser.Error( offset, - Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err().toList)) + Parser.Expectation.unify(NonEmptyList.fromListUnsafe(err.value.toList)) ) ) } @@ -1783,7 +1783,7 @@ object Parser { */ protected[parse] final class State(val str: String) { var offset: Int = 0 - var error: () => Chain[Expectation] = null + var error: Eval[Chain[Expectation]] = null var capture: Boolean = true } @@ -2043,7 +2043,8 @@ object Parser { state.offset = end res } else { - state.error = () => Chain.one(Expectation.Length(offset, len, state.str.length - offset)) + state.error = + Eval.later(Chain.one(Expectation.Length(offset, len, state.str.length - offset))) null } } @@ -2091,7 +2092,7 @@ object Parser { override def parseMut(state: State): Unit = { val offset = state.offset if (offset != 0) { - state.error = () => (Chain.one(Expectation.StartOfString(offset))) + state.error = Eval.later(Chain.one(Expectation.StartOfString(offset))) } () } @@ -2101,7 +2102,7 @@ object Parser { override def parseMut(state: State): Unit = { val offset = state.offset if (offset != state.str.length) { - state.error = () => (Chain.one(Expectation.EndOfString(offset, state.str.length))) + state.error = Eval.later(Chain.one(Expectation.EndOfString(offset, state.str.length))) } () } @@ -2140,7 +2141,7 @@ object Parser { state.offset += message.length () } else { - state.error = () => (Chain.one(Expectation.OneOfStr(offset, message :: Nil))) + state.error = Eval.later(Chain.one(Expectation.OneOfStr(offset, message :: Nil))) () } } @@ -2156,7 +2157,7 @@ object Parser { state.offset += message.length () } else { - state.error = () => (Chain.one(Expectation.OneOfStr(offset, message :: Nil))) + state.error = Eval.later(Chain.one(Expectation.OneOfStr(offset, message :: Nil))) () } } @@ -2165,7 +2166,7 @@ object Parser { case class Fail[A]() extends Parser[A] { override def parseMut(state: State): A = { val offset = state.offset - state.error = () => (Chain.one(Expectation.Fail(offset))) + state.error = Eval.later(Chain.one(Expectation.Fail(offset))) null.asInstanceOf[A] } } @@ -2173,14 +2174,14 @@ object Parser { case class FailWith[A](message: String) extends Parser[A] { override def parseMut(state: State): A = { val offset = state.offset - state.error = () => (Chain.one(Expectation.FailWith(offset, message))) + state.error = Eval.later(Chain.one(Expectation.FailWith(offset, message))) null.asInstanceOf[A] } } final def oneOf[A](all: Array[Parser0[A]], state: State): A = { val offset = state.offset - var errs: () => Chain[Expectation] = () => Chain.nil + var errs: Eval[Chain[Expectation]] = Eval.later(Chain.nil) var idx = 0 while (idx < all.length) { val thisParser = all(idx) @@ -2194,7 +2195,7 @@ object Parser { // we failed to parse, but didn't consume input // is unchanged we continue // else we stop - errs = () => errs() ++ err() + errs = errs.map(_ ++ err.value) state.error = null idx = idx + 1 } @@ -2235,7 +2236,7 @@ object Parser { } } if (lastMatch < 0) { - state.error = () => (Chain.one(Expectation.OneOfStr(startOffset, all.toList))) + state.error = Eval.later(Chain.one(Expectation.OneOfStr(startOffset, all.toList))) state.offset = startOffset } else { state.offset = lastMatch @@ -2659,7 +2660,8 @@ object Parser { state.offset += 1 char } else { - state.error = () => (Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) + state.error = + Eval.later(Chain.one(Expectation.InRange(offset, Char.MinValue, Char.MaxValue))) '\u0000' } } @@ -2670,8 +2672,8 @@ object Parser { override def toString = s"CharIn($min, bitSet = ..., $ranges)" - def makeError(offset: Int): () => Chain[Expectation] = { () => - ( + def makeError(offset: Int): Eval[Chain[Expectation]] = { + Eval.later( Chain.fromSeq( ranges.toList.map { case (s, e) => Expectation.InRange(offset, s, e) @@ -2716,7 +2718,7 @@ object Parser { val matchedStr = state.str.substring(offset, state.offset) // we don't reset the offset, so if the underlying parser // advanced it will fail in a OneOf - state.error = () => (Chain.one(Expectation.ExpectedFailureAt(offset, matchedStr))) + state.error = Eval.later(Chain.one(Expectation.ExpectedFailureAt(offset, matchedStr))) } state.offset = offset From bbfe4d71520ba6328b4cf4cd88fcb582b4911df8 Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Thu, 14 Oct 2021 23:46:16 -0400 Subject: [PATCH 5/6] Add MiMa filter for CharIn.makeError --- build.sbt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index f343ac11..86fdb93f 100644 --- a/build.sbt +++ b/build.sbt @@ -1,3 +1,4 @@ +import com.typesafe.tools.mima.core._ import sbtcrossproject.CrossPlugin.autoImport.{crossProject, CrossType} import Dependencies._ @@ -133,7 +134,12 @@ lazy val core = crossProject(JSPlatform, JVMPlatform) mimaPreviousArtifacts := { val isScala211 = CrossVersion.partialVersion(scalaVersion.value).contains((2, 11)) if (isScala211) Set.empty else mimaPreviousArtifacts.value - } + }, + mimaBinaryIssueFilters := Seq( + ProblemFilters.exclude[IncompatibleResultTypeProblem]( + "cats.parse.Parser#Impl#CharIn.makeError" + ) // Impl is private to cats.parse.Parser + ) ) .jsSettings( crossScalaVersions := (ThisBuild / crossScalaVersions).value.filterNot(_.startsWith("2.11")), From 9561249143c464ad2c1d25bc1a2868cd67e409b2 Mon Sep 17 00:00:00 2001 From: "Ross A. Baker" Date: Fri, 15 Oct 2021 11:05:43 -0400 Subject: [PATCH 6/6] Restore optimization from #62 --- .../src/main/scala/cats/parse/Parser.scala | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala index 9d978fe1..d1228aa3 100644 --- a/core/shared/src/main/scala/cats/parse/Parser.scala +++ b/core/shared/src/main/scala/cats/parse/Parser.scala @@ -2673,13 +2673,16 @@ object Parser { override def toString = s"CharIn($min, bitSet = ..., $ranges)" def makeError(offset: Int): Eval[Chain[Expectation]] = { - Eval.later( - Chain.fromSeq( - ranges.toList.map { case (s, e) => - Expectation.InRange(offset, s, e) - } - ) - ) + Eval.later { + var result = Chain.empty[Expectation] + var aux = ranges.toList + while (aux.nonEmpty) { + val (s, e) = aux.head + result = result :+ Expectation.InRange(offset, s, e) + aux = aux.tail + } + result + } } override def parseMut(state: State): Char = {