#248 Backport (#249)

Backports #248 to 4.5, fixing #241 properly.

.github/workflows/ci.yml

@@ -24,10 +24,10 @@ concurrency:
 
 jobs:
   build:
-    name: Build and Test
+    name: Test
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-22.04]
         scala: [2.13, 2.12, 3]
         java: [temurin@8, temurin@11, temurin@17]
         project: [rootJS, rootJVM, rootNative]
@@ -51,15 +51,14 @@ jobs:
     runs-on: ${{ matrix.os }}
     timeout-minutes: 60
     steps:
-      - name: Install sbt
-        if: contains(runner.os, 'macos')
-        run: brew install sbt
-
       - name: Checkout current branch (full)
         uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
+      - name: Setup sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'
@@ -103,7 +102,7 @@ jobs:
         run: sbt githubWorkflowCheck
 
       - name: Check headers
-        if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
+        if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-22.04'
         run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' headerCheckAll
 
       - name: scalaJSLink
@@ -118,11 +117,11 @@ jobs:
         run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' test
 
       - name: Check binary compatibility
-        if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
+        if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-22.04'
         run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' mimaReportBinaryIssues
 
       - name: Generate API documentation
-        if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
+        if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-22.04'
         run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' doc
 
       - name: Make target directories
@@ -146,19 +145,18 @@ jobs:
     if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/master')
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-22.04]
         java: [temurin@8]
     runs-on: ${{ matrix.os }}
     steps:
-      - name: Install sbt
-        if: contains(runner.os, 'macos')
-        run: brew install sbt
-
       - name: Checkout current branch (full)
         uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
+      - name: Setup sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'
@@ -317,19 +315,18 @@ jobs:
     if: github.event.repository.fork == false && github.event_name != 'pull_request'
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-22.04]
         java: [temurin@8]
     runs-on: ${{ matrix.os }}
     steps:
-      - name: Install sbt
-        if: contains(runner.os, 'macos')
-        run: brew install sbt
-
       - name: Checkout current branch (full)
         uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
+      - name: Setup sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'
@@ -380,14 +377,17 @@ jobs:
     if: github.ref == 'refs/heads/master' || (github.event_name == 'pull_request' && github.base_ref == 'master')
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-22.04]
         scala: [2.13]
         java: [temurin@11]
     runs-on: ${{ matrix.os }}
     steps:
       - name: Checkout current branch (fast)
         uses: actions/checkout@v4
 
+      - name: Setup sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Setup Java (temurin@11)
         id: setup-java-temurin-11
         if: matrix.java == 'temurin@11'
@@ -417,19 +417,18 @@ jobs:
     name: Generate Site
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-22.04]
         java: [temurin@11]
     runs-on: ${{ matrix.os }}
     steps:
-      - name: Install sbt
-        if: contains(runner.os, 'macos')
-        run: brew install sbt
-
       - name: Checkout current branch (full)
         uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
+      - name: Setup sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'

README.md

@@ -9,18 +9,18 @@ Parsley is distributed on Maven Central, and can be added to your project via:
 
 ```scala
 // SBT
-libraryDependencies += "com.github.j-mie6" %% "parsley" % "4.5.2"
+libraryDependencies += "com.github.j-mie6" %% "parsley" % "4.5.3"
 
 // scala-cli
---dependency com.github.j-mie6::parsley:4.5.2
+--dependency com.github.j-mie6::parsley:4.5.3
 // or in file
-//> using dep com.github.j-mie6::parsley:4.5.2
+//> using dep com.github.j-mie6::parsley:4.5.3
 
 // mill
-ivy"com.github.j-mie6::parsley:4.5.2"
+ivy"com.github.j-mie6::parsley:4.5.3"
 ```
 
-Documentation can be found [**here**](https://javadoc.io/doc/com.github.j-mie6/parsley_2.13/latest/index.html)
+Documentation can be found [**here**](https://javadoc.io/doc/com.github.j-mie6/parsley-docs_2.13/latest/index.html)
 
 If you're a `cats` user, you may also be interested in using [`parsley-cats`](https://github.com/j-mie6/parsley-cats)<a href="https://typelevel.org/cats/"><img src="https://typelevel.org/cats/img/cats-badge.svg" height="40px" align="right" alt="Cats friendly" /></a>
 to augment `parsley` with instances for various `cats` typeclasses:

build.sbt

@@ -126,6 +126,7 @@ def testCoverageJob(cacheSteps: List[WorkflowStep]) = WorkflowJob(
     cond = Some(s"github.ref == 'refs/heads/$mainBranch' || (github.event_name == 'pull_request' && github.base_ref == '$mainBranch')"),
     steps =
         WorkflowStep.Checkout ::
+        WorkflowStep.SetupSbt ::
         WorkflowStep.SetupJava(List(JavaLTS)) :::
         cacheSteps ::: List(
             WorkflowStep.Sbt(name = Some("Generate coverage report"), commands = List("coverage", "parsley / test", "parsleyDebug / test", "coverageReport")),

parsley/shared/src/main/scala/parsley/internal/deepembedding/backend/SequenceEmbedding.scala

@@ -93,10 +93,9 @@ private [deepembedding] final class >>=[A, B](val p: StrictParsley[A], private [
     }
     override def codeGen[M[_, +_]: ContOps, R](producesResults: Boolean)(implicit instrs: InstrBuffer, state: CodeGenState): M[R, Unit] = {
         suspend(p.codeGen[M, R](producesResults = true)) |> {
-            instrs += instructions.DynCall[A] { x =>
+            instrs += instructions.DynCall[A] { (x, refsSz) =>
                 val p = f(x)
-                // FIXME: suppress results within p, then can remove pop
-                p.demandCalleeSave(state.numRegs)
+                p.setMinReferenceAllocation(refsSz)
                 if (implicitly[ContOps[M]].isStackSafe) p.overflows()
                 p.instrs
             }

parsley/shared/src/main/scala/parsley/internal/deepembedding/backend/StrictParsley.scala

@@ -16,7 +16,7 @@ import parsley.internal.deepembedding.ContOps, ContOps.{perform, ContAdapter}
 import parsley.internal.machine.instructions, instructions.{Instr, Label}
 
 import StrictParsley.*
-import org.typelevel.scalaccompat.annotation.{nowarn, nowarn3}
+import org.typelevel.scalaccompat.annotation.nowarn3
 
 /** This is the root type of the parsley "backend": it represents a combinator tree
   * where the join-points in the tree (recursive or otherwise) have been factored into
@@ -44,14 +44,15 @@ private [deepembedding] trait StrictParsley[+A] {
       * @param state the code generator state
       * @return the final array of instructions for this parser
       */
-    final private [deepembedding] def generateInstructions[M[_, +_]: ContOps](numRegsUsedByParent: Int, usedRefs: Set[Ref[_]],
+    final private [deepembedding] def generateInstructions[M[_, +_]: ContOps](minRef: Int, usedRefs: Set[Ref[_]],
                                                                               bodyMap: Map[Let[_], StrictParsley[_]])
                                                                             (implicit state: CodeGenState): Array[Instr] = {
         implicit val instrs: InstrBuffer = newInstrBuffer
         perform {
-            generateCalleeSave[M, Array[Instr]](numRegsUsedByParent, this.codeGen(producesResults = true), usedRefs) |> {
-                // When `numRegsUsedByParent` is -1 this is top level, otherwise it is a flatMap
-                instrs += (if (numRegsUsedByParent >= 0) instructions.Return else instructions.Halt)
+            allocateAndExpandRefs(minRef, usedRefs)
+            this.codeGen[M, Array[Instr]](producesResults = true) |> {
+                // When `minRef` is -1 this is top level, otherwise it is a flatMap
+                instrs += (if (minRef >= 0) instructions.Return else instructions.Halt)
                 val letRets = finaliseLets(bodyMap)
                 generateHandlers(state.handlers)
                 finaliseInstrs(instrs, state.nlabels, letRets)
@@ -98,51 +99,6 @@ private [deepembedding] object StrictParsley {
     /** Make a fresh instruction buffer */
     private def newInstrBuffer: InstrBuffer = new ResizableArray()
 
-    /** Given a set of in-use registers, this function will allocate those that are currented
-      * unallocated, giving them addresses not currently in use by the allocated registers
-      *
-      * @param unallocatedRegs the set of registers that need allocating
-      * @param regs the set of all registers used by a specific parser
-      * @return the list of slots that have been freshly allocated to
-      */
-    private def allocateRegisters(unallocatedRegs: Set[Ref[_]], regs: Set[Ref[_]]): List[Int] = {
-        // Global registers cannot occupy the same slot as another global register
-        // In a flatMap, that means a newly discovered global register must be allocated to a new slot: this may resize the register pool
-        assert(unallocatedRegs == regs.filterNot(_.allocated))
-        if (unallocatedRegs.nonEmpty) {
-            val usedSlots = regs.collect {
-                case reg if reg.allocated => reg.addr
-            }: @nowarn
-            val freeSlots = (0 until regs.size).filterNot(usedSlots)
-            applyAllocation(unallocatedRegs, freeSlots)
-        }
-        else Nil
-    }
-
-    /** Given a set of unallocated registers and a supply of unoccupied slots, allocates each
-      * register to one of the slots.
-      *
-      * @param regs the set of registers that require allocation
-      * @param freeSlots the supply of slots that are currently not in-use
-      * @return the slots that were used for allocation
-      */
-    private def applyAllocation(refs: Set[Ref[_]] @nowarn3, freeSlots: Iterable[Int]): List[Int] = {
-        val allocatedSlots = mutable.ListBuffer.empty[Int]
-        // TODO: For scala 2.12, use lazyZip and foreach!
-        /*for ((ref, addr) <- refs.zip(freeSlots)) {
-            ref.allocate(addr)
-            allocatedSlots += addr
-        }*/ // FIXME: until 5.0.0 we need to suppress warnings, and Scala 3 is being annoying (refreshing change)
-        type Ref_ = Ref[_]
-        refs.zip(freeSlots).foreach { (refAndAddr: (Ref[_], Int) @nowarn3) =>
-            val ref: Ref_ @nowarn3 = refAndAddr._1
-            val addr = refAndAddr._2
-            ref.allocate(addr)
-            allocatedSlots += addr
-        }
-        allocatedSlots.toList
-    }
-
     /** If required, generates callee-save around a main body of instructions.
       *
       * This is needed when using `flatMap`, as it is unaware of the register
@@ -160,23 +116,19 @@ private [deepembedding] object StrictParsley {
       * @param instrs the instruction buffer
       * @param state the code generation state, for label generation
       */
-    private def generateCalleeSave[M[_, +_]: ContOps, R](numRegsUsedByParent: Int, bodyGen: =>M[R, Unit], usedRefs: Set[Ref[_]])
-                                                       (implicit instrs: InstrBuffer, state: CodeGenState): M[R, Unit] = {
-        val reqRegs = usedRefs.size
-        val localRegs: Set[Ref[_]] @nowarn3 = usedRefs.filterNot(_.allocated): @nowarn3
-        val allocatedRegs = allocateRegisters(localRegs, usedRefs)
-        val calleeSaveRequired = numRegsUsedByParent >= 0 // if this is -1, then we are the top level and have no parent, otherwise it needs to be done
-        if (calleeSaveRequired && localRegs.nonEmpty) {
-            val end = state.freshLabel()
-            val calleeSave = state.freshLabel()
-            instrs += new instructions.Label(calleeSave)
-            instrs += new instructions.CalleeSave(end, localRegs, reqRegs, allocatedRegs, numRegsUsedByParent)
-            bodyGen |> {
-                instrs += new instructions.Jump(calleeSave)
-                instrs += new instructions.Label(end)
+    private def allocateAndExpandRefs(minRef: Int, usedRefs: Set[Ref[_]])(implicit instrs: InstrBuffer): Unit = {
+        var nextSlot = math.max(minRef, 0)
+        usedRefs.foreach { (r: Ref[_]) =>
+            if (!r.allocated) {
+                r.allocate(nextSlot)
+                nextSlot += 1
             }
         }
-        else bodyGen
+        val totalSlotsRequired = nextSlot
+        // if this is -1, then we are the top level and have no parent, otherwise it needs to be done
+        if (minRef >= 0 && (minRef < totalSlotsRequired)) {
+            instrs += new instructions.ExpandRefs(totalSlotsRequired)
+        }
     }
 
     /** Generates each of the shared, non-recursive, parsers that have been ''used'' by

parsley/shared/src/main/scala/parsley/internal/deepembedding/frontend/LazyParsley.scala

@@ -44,12 +44,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
     // The instructions used to execute this parser along with the number of registers it uses
     final private [parsley] lazy val (instrs: Array[Instr], numRegs: Int) = computeInstrs
 
-    /** This parser is the result of a `flatMap` operation, and as such must perform
-      * callee-save on `numRegs` registers (which belong to its parent)
-      *
-      * @param numRegs the number of registers the parent uses (these must be saved)
-      */
-    private [deepembedding] def demandCalleeSave(numRegs: Int): Unit = numRegsUsedByParent = numRegs
+    private [deepembedding] def setMinReferenceAllocation(minRef: Int): Unit = this.minRef = minRef
 
     // Internals
     // To ensure that stack-overflow cannot occur during the processing of particularly
@@ -87,7 +82,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
     final private var cps = false
     final private [deepembedding] def isCps: Boolean = cps
     /** how many registers are used by the ''parent'' of this combinator (this combinator is part of a `flatMap` when this is not -1) */
-    final private var numRegsUsedByParent = -1
+    final private var minRef = -1
 
     /** Computes the instructions associated with this parser as well as the number of
       * registers it requires in a (possibly) stack-safe way.
@@ -120,7 +115,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
                 implicit val letMap: LetMap = LetMap(letFinderState.lets, letFinderState.recs)
                 for { sp <- this.optimised } yield {
                     implicit val state: backend.CodeGenState = new backend.CodeGenState(letFinderState.numRegs)
-                    sp.generateInstructions(numRegsUsedByParent, usedRefs, letMap.bodies)
+                    sp.generateInstructions(minRef, usedRefs, letMap.bodies)
                 }
             }
         }, letFinderState.numRegs)

parsley/shared/src/main/scala/parsley/internal/machine/instructions/CoreInstrs.scala

@@ -72,17 +72,17 @@ private [internal] object Apply extends Instr {
 }
 
 // Monadic
-private [internal] final class DynCall(f: Any => Array[Instr]) extends Instr {
+private [internal] final class DynCall(f: (Any, Int) => Array[Instr]) extends Instr {
     override def apply(ctx: Context): Unit = {
         ensureRegularInstruction(ctx)
-        ctx.call(f(ctx.stack.upop()))
+        ctx.call(f(ctx.stack.upop(), ctx.regs.size))
     }
     // $COVERAGE-OFF$
     override def toString: String = "DynCall(?)"
     // $COVERAGE-ON$
 }
 private [internal] object DynCall {
-    def apply[A](f: A => Array[Instr]): DynCall = new DynCall(f.asInstanceOf[Any => Array[Instr]])
+    def apply[A](f: (A, Int) => Array[Instr]): DynCall = new DynCall(f.asInstanceOf[(Any, Int) => Array[Instr]])
 }
 
 // Control Flow