improvement: Try and remove more macros from parsers #3359

tgodzik · 2023-10-03T16:40:51Z

Also some changes to make it cross compile

tgodzik · 2023-10-03T17:27:04Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScaladocParser.scala

-  private def hspaces0[_: P] = hspacesMin(0)
-  private def hspaces1[_: P] = hspacesMin(1)
-  private def hspacesMinWithLen[_: P](min: Int): P[Int] =
+  private def hspace[$: P] = CharIn("\t\r ")


Looks like the previous breaks in Scala 3 and it's also changed in the examples of fastparse https://com-lihaoyi.github.io/fastparse/

tgodzik · 2023-10-03T17:27:26Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

-    do {
+
+    builder += parseRuleAfterBOF(parseSourceImpl())
+    while (in.token match {


There is no do ... while() construct for Scala 3

since we have it in many places, and since in at least one you define a method, why not define our own:

// in companion object ScalametaParser, or perhaps even in scala.meta.common somewhere def doWhile(body: => Unit)(cond: => Boolean): Unit = { body while (cond) body }

Good idea, done!

tgodzik · 2023-10-03T17:28:08Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

@@ -196,6 +198,10 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
  @inline private def tryAheadNot[T: ClassTag]: Boolean =
    nextIf(!peekToken.is[T])

+  private def unreachable(debugges: Map[String, Any]): Nothing = UnreachableError.raise(debugges)


Another set of macros that were useful, but were possible to replace

debugges? typo?

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

tgodzik · 2023-10-03T17:29:26Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

@@ -583,7 +594,7 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
    // Unquote's pt may not be directly equal unwrapped ellipsis's pt, but be its refinement instead.
    // For example, in `new { ..$stats }`, ellipsis's pt is List[Stat], but quasi's pt is Term.
    // This is an artifact of the current implementation, so we just need to keep it mind and work around it.
-    require(classTag[T].runtimeClass.isAssignableFrom(tree.pt) && debug(ell, tree, tree.structure))
+    assert(classTag[T].runtimeClass.isAssignableFrom(tree.pt) && debug(ell, tree, tree.structure))


require is another macro

tgodzik · 2023-10-03T17:30:08Z

scalameta/parsers/shared/src/main/scala/scala/meta/parsers/Errors.scala

@@ -6,7 +6,7 @@ import org.scalameta.data._
 import scala.meta.inputs._
 import scala.meta.internal.inputs._

-@root trait Parsed[+T] {


Not sure why this needed to be marked as ast since this are perfectly fine as case classes.

kitbellew · 2023-10-03T20:23:08Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

@@ -196,6 +198,10 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
  @inline private def tryAheadNot[T: ClassTag]: Boolean =
    nextIf(!peekToken.is[T])

+  private def unreachable(debugges: Map[String, Any]): Nothing = UnreachableError.raise(debugges)


debugges? typo?

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

kitbellew · 2023-10-03T20:26:23Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

@@ -1875,7 +1889,7 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
    }).map(_.reduceWith(toParamClause(None)))

  def implicitClosure(location: Location): Term.Function = {
-    require(token.isNot[KwImplicit] && debug(token))
+    assert(token.isNot[KwImplicit], s"Expected implicit keyword but found ${token.name}")


actually, it's the opposite. expected to be after the implicit keyword... why not make implicitClosure private and remove the assert?

Done! That's much better, I don't love putting asserts anywhere in the code aside from tests.

kitbellew · 2023-10-03T20:38:50Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

-    do {
+
+    builder += parseRuleAfterBOF(parseSourceImpl())
+    while (in.token match {


since we have it in many places, and since in at least one you define a method, why not define our own:

// in companion object ScalametaParser, or perhaps even in scala.meta.common somewhere def doWhile(body: => Unit)(cond: => Boolean): Unit = { body while (cond) body }

kitbellew · 2023-10-04T12:55:25Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

-    })
+
+    doWhile {
+      parseRuleAfterBOF(parseSourceImpl())


builder + ?

kitbellew · 2023-10-04T14:37:02Z

scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala

-    })
+
+    doWhile {
+     builder += parseRuleAfterBOF(parseSourceImpl())


Suggested change

builder += parseRuleAfterBOF(parseSourceImpl())

builder += parseRuleAfterBOF(parseSourceImpl())

I was too quick with pushing 😅

tgodzik · 2023-10-04T15:10:14Z

Ok, it's good now 😅

tgodzik force-pushed the remove-macro2 branch 2 times, most recently from 63433bc to 7f153d5 Compare October 3, 2023 17:26

tgodzik commented Oct 3, 2023

View reviewed changes

improvement: Try and remove more macros from parsers

cf61003

tgodzik force-pushed the remove-macro2 branch from 7f153d5 to cf61003 Compare October 3, 2023 17:41

tgodzik marked this pull request as ready for review October 3, 2023 17:41

tgodzik requested a review from kitbellew October 3, 2023 17:41

kitbellew requested changes Oct 3, 2023

View reviewed changes

tgodzik requested a review from kitbellew October 4, 2023 12:19

kitbellew requested changes Oct 4, 2023

View reviewed changes

tgodzik force-pushed the remove-macro2 branch from 63a96da to 8db27a1 Compare October 4, 2023 14:28

kitbellew reviewed Oct 4, 2023

View reviewed changes

improvement: Change implicits to private, implement doWhile, fix typo.

72bb3b0

tgodzik force-pushed the remove-macro2 branch from 8db27a1 to 72bb3b0 Compare October 4, 2023 14:46

tgodzik requested a review from kitbellew October 4, 2023 15:10

kitbellew merged commit 69f5292 into scalameta:main Oct 4, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improvement: Try and remove more macros from parsers #3359

improvement: Try and remove more macros from parsers #3359

tgodzik commented Oct 3, 2023 •

edited

tgodzik Oct 3, 2023

tgodzik Oct 3, 2023

kitbellew Oct 3, 2023

tgodzik Oct 4, 2023

tgodzik Oct 3, 2023

kitbellew Oct 3, 2023

tgodzik Oct 4, 2023

tgodzik Oct 3, 2023

tgodzik Oct 3, 2023

kitbellew Oct 3, 2023

kitbellew Oct 3, 2023

tgodzik Oct 4, 2023

kitbellew Oct 3, 2023

kitbellew Oct 4, 2023

kitbellew Oct 4, 2023

tgodzik Oct 4, 2023

tgodzik commented Oct 4, 2023

	builder += parseRuleAfterBOF(parseSourceImpl())
	builder += parseRuleAfterBOF(parseSourceImpl())

improvement: Try and remove more macros from parsers #3359

improvement: Try and remove more macros from parsers #3359

Conversation

tgodzik commented Oct 3, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgodzik commented Oct 4, 2023

tgodzik commented Oct 3, 2023 •

edited