Skip to content

Commit 1af1ee9

Browse files
authored
#248 Backport (#249)
Backports #248 to 4.5, fixing #241 properly.
2 parents 188a941 + 3a37283 commit 1af1ee9

File tree

12 files changed

+67
-183
lines changed

12 files changed

+67
-183
lines changed

.github/workflows/ci.yml

+24-25
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ concurrency:
2424

2525
jobs:
2626
build:
27-
name: Build and Test
27+
name: Test
2828
strategy:
2929
matrix:
30-
os: [ubuntu-latest]
30+
os: [ubuntu-22.04]
3131
scala: [2.13, 2.12, 3]
3232
java: [temurin@8, temurin@11, temurin@17]
3333
project: [rootJS, rootJVM, rootNative]
@@ -51,15 +51,14 @@ jobs:
5151
runs-on: ${{ matrix.os }}
5252
timeout-minutes: 60
5353
steps:
54-
- name: Install sbt
55-
if: contains(runner.os, 'macos')
56-
run: brew install sbt
57-
5854
- name: Checkout current branch (full)
5955
uses: actions/checkout@v4
6056
with:
6157
fetch-depth: 0
6258

59+
- name: Setup sbt
60+
uses: sbt/setup-sbt@v1
61+
6362
- name: Setup Java (temurin@8)
6463
id: setup-java-temurin-8
6564
if: matrix.java == 'temurin@8'
@@ -103,7 +102,7 @@ jobs:
103102
run: sbt githubWorkflowCheck
104103

105104
- name: Check headers
106-
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
105+
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-22.04'
107106
run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' headerCheckAll
108107

109108
- name: scalaJSLink
@@ -118,11 +117,11 @@ jobs:
118117
run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' test
119118

120119
- name: Check binary compatibility
121-
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
120+
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-22.04'
122121
run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' mimaReportBinaryIssues
123122

124123
- name: Generate API documentation
125-
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
124+
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-22.04'
126125
run: sbt 'project ${{ matrix.project }}' '++ ${{ matrix.scala }}' doc
127126

128127
- name: Make target directories
@@ -146,19 +145,18 @@ jobs:
146145
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/master')
147146
strategy:
148147
matrix:
149-
os: [ubuntu-latest]
148+
os: [ubuntu-22.04]
150149
java: [temurin@8]
151150
runs-on: ${{ matrix.os }}
152151
steps:
153-
- name: Install sbt
154-
if: contains(runner.os, 'macos')
155-
run: brew install sbt
156-
157152
- name: Checkout current branch (full)
158153
uses: actions/checkout@v4
159154
with:
160155
fetch-depth: 0
161156

157+
- name: Setup sbt
158+
uses: sbt/setup-sbt@v1
159+
162160
- name: Setup Java (temurin@8)
163161
id: setup-java-temurin-8
164162
if: matrix.java == 'temurin@8'
@@ -317,19 +315,18 @@ jobs:
317315
if: github.event.repository.fork == false && github.event_name != 'pull_request'
318316
strategy:
319317
matrix:
320-
os: [ubuntu-latest]
318+
os: [ubuntu-22.04]
321319
java: [temurin@8]
322320
runs-on: ${{ matrix.os }}
323321
steps:
324-
- name: Install sbt
325-
if: contains(runner.os, 'macos')
326-
run: brew install sbt
327-
328322
- name: Checkout current branch (full)
329323
uses: actions/checkout@v4
330324
with:
331325
fetch-depth: 0
332326

327+
- name: Setup sbt
328+
uses: sbt/setup-sbt@v1
329+
333330
- name: Setup Java (temurin@8)
334331
id: setup-java-temurin-8
335332
if: matrix.java == 'temurin@8'
@@ -380,14 +377,17 @@ jobs:
380377
if: github.ref == 'refs/heads/master' || (github.event_name == 'pull_request' && github.base_ref == 'master')
381378
strategy:
382379
matrix:
383-
os: [ubuntu-latest]
380+
os: [ubuntu-22.04]
384381
scala: [2.13]
385382
java: [temurin@11]
386383
runs-on: ${{ matrix.os }}
387384
steps:
388385
- name: Checkout current branch (fast)
389386
uses: actions/checkout@v4
390387

388+
- name: Setup sbt
389+
uses: sbt/setup-sbt@v1
390+
391391
- name: Setup Java (temurin@11)
392392
id: setup-java-temurin-11
393393
if: matrix.java == 'temurin@11'
@@ -417,19 +417,18 @@ jobs:
417417
name: Generate Site
418418
strategy:
419419
matrix:
420-
os: [ubuntu-latest]
420+
os: [ubuntu-22.04]
421421
java: [temurin@11]
422422
runs-on: ${{ matrix.os }}
423423
steps:
424-
- name: Install sbt
425-
if: contains(runner.os, 'macos')
426-
run: brew install sbt
427-
428424
- name: Checkout current branch (full)
429425
uses: actions/checkout@v4
430426
with:
431427
fetch-depth: 0
432428

429+
- name: Setup sbt
430+
uses: sbt/setup-sbt@v1
431+
433432
- name: Setup Java (temurin@8)
434433
id: setup-java-temurin-8
435434
if: matrix.java == 'temurin@8'

README.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,18 @@ Parsley is distributed on Maven Central, and can be added to your project via:
99

1010
```scala
1111
// SBT
12-
libraryDependencies += "com.github.j-mie6" %% "parsley" % "4.5.2"
12+
libraryDependencies += "com.github.j-mie6" %% "parsley" % "4.5.3"
1313

1414
// scala-cli
15-
--dependency com.github.j-mie6::parsley:4.5.2
15+
--dependency com.github.j-mie6::parsley:4.5.3
1616
// or in file
17-
//> using dep com.github.j-mie6::parsley:4.5.2
17+
//> using dep com.github.j-mie6::parsley:4.5.3
1818

1919
// mill
20-
ivy"com.github.j-mie6::parsley:4.5.2"
20+
ivy"com.github.j-mie6::parsley:4.5.3"
2121
```
2222

23-
Documentation can be found [**here**](https://javadoc.io/doc/com.github.j-mie6/parsley_2.13/latest/index.html)
23+
Documentation can be found [**here**](https://javadoc.io/doc/com.github.j-mie6/parsley-docs_2.13/latest/index.html)
2424

2525
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>
2626
to augment `parsley` with instances for various `cats` typeclasses:

build.sbt

+1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ def testCoverageJob(cacheSteps: List[WorkflowStep]) = WorkflowJob(
126126
cond = Some(s"github.ref == 'refs/heads/$mainBranch' || (github.event_name == 'pull_request' && github.base_ref == '$mainBranch')"),
127127
steps =
128128
WorkflowStep.Checkout ::
129+
WorkflowStep.SetupSbt ::
129130
WorkflowStep.SetupJava(List(JavaLTS)) :::
130131
cacheSteps ::: List(
131132
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

+2-3
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,9 @@ private [deepembedding] final class >>=[A, B](val p: StrictParsley[A], private [
9393
}
9494
override def codeGen[M[_, +_]: ContOps, R](producesResults: Boolean)(implicit instrs: InstrBuffer, state: CodeGenState): M[R, Unit] = {
9595
suspend(p.codeGen[M, R](producesResults = true)) |> {
96-
instrs += instructions.DynCall[A] { x =>
96+
instrs += instructions.DynCall[A] { (x, refsSz) =>
9797
val p = f(x)
98-
// FIXME: suppress results within p, then can remove pop
99-
p.demandCalleeSave(state.numRegs)
98+
p.setMinReferenceAllocation(refsSz)
10099
if (implicitly[ContOps[M]].isStackSafe) p.overflows()
101100
p.instrs
102101
}

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

+17-65
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import parsley.internal.deepembedding.ContOps, ContOps.{perform, ContAdapter}
1616
import parsley.internal.machine.instructions, instructions.{Instr, Label}
1717

1818
import StrictParsley.*
19-
import org.typelevel.scalaccompat.annotation.{nowarn, nowarn3}
19+
import org.typelevel.scalaccompat.annotation.nowarn3
2020

2121
/** This is the root type of the parsley "backend": it represents a combinator tree
2222
* where the join-points in the tree (recursive or otherwise) have been factored into
@@ -44,14 +44,15 @@ private [deepembedding] trait StrictParsley[+A] {
4444
* @param state the code generator state
4545
* @return the final array of instructions for this parser
4646
*/
47-
final private [deepembedding] def generateInstructions[M[_, +_]: ContOps](numRegsUsedByParent: Int, usedRefs: Set[Ref[_]],
47+
final private [deepembedding] def generateInstructions[M[_, +_]: ContOps](minRef: Int, usedRefs: Set[Ref[_]],
4848
bodyMap: Map[Let[_], StrictParsley[_]])
4949
(implicit state: CodeGenState): Array[Instr] = {
5050
implicit val instrs: InstrBuffer = newInstrBuffer
5151
perform {
52-
generateCalleeSave[M, Array[Instr]](numRegsUsedByParent, this.codeGen(producesResults = true), usedRefs) |> {
53-
// When `numRegsUsedByParent` is -1 this is top level, otherwise it is a flatMap
54-
instrs += (if (numRegsUsedByParent >= 0) instructions.Return else instructions.Halt)
52+
allocateAndExpandRefs(minRef, usedRefs)
53+
this.codeGen[M, Array[Instr]](producesResults = true) |> {
54+
// When `minRef` is -1 this is top level, otherwise it is a flatMap
55+
instrs += (if (minRef >= 0) instructions.Return else instructions.Halt)
5556
val letRets = finaliseLets(bodyMap)
5657
generateHandlers(state.handlers)
5758
finaliseInstrs(instrs, state.nlabels, letRets)
@@ -98,51 +99,6 @@ private [deepembedding] object StrictParsley {
9899
/** Make a fresh instruction buffer */
99100
private def newInstrBuffer: InstrBuffer = new ResizableArray()
100101

101-
/** Given a set of in-use registers, this function will allocate those that are currented
102-
* unallocated, giving them addresses not currently in use by the allocated registers
103-
*
104-
* @param unallocatedRegs the set of registers that need allocating
105-
* @param regs the set of all registers used by a specific parser
106-
* @return the list of slots that have been freshly allocated to
107-
*/
108-
private def allocateRegisters(unallocatedRegs: Set[Ref[_]], regs: Set[Ref[_]]): List[Int] = {
109-
// Global registers cannot occupy the same slot as another global register
110-
// In a flatMap, that means a newly discovered global register must be allocated to a new slot: this may resize the register pool
111-
assert(unallocatedRegs == regs.filterNot(_.allocated))
112-
if (unallocatedRegs.nonEmpty) {
113-
val usedSlots = regs.collect {
114-
case reg if reg.allocated => reg.addr
115-
}: @nowarn
116-
val freeSlots = (0 until regs.size).filterNot(usedSlots)
117-
applyAllocation(unallocatedRegs, freeSlots)
118-
}
119-
else Nil
120-
}
121-
122-
/** Given a set of unallocated registers and a supply of unoccupied slots, allocates each
123-
* register to one of the slots.
124-
*
125-
* @param regs the set of registers that require allocation
126-
* @param freeSlots the supply of slots that are currently not in-use
127-
* @return the slots that were used for allocation
128-
*/
129-
private def applyAllocation(refs: Set[Ref[_]] @nowarn3, freeSlots: Iterable[Int]): List[Int] = {
130-
val allocatedSlots = mutable.ListBuffer.empty[Int]
131-
// TODO: For scala 2.12, use lazyZip and foreach!
132-
/*for ((ref, addr) <- refs.zip(freeSlots)) {
133-
ref.allocate(addr)
134-
allocatedSlots += addr
135-
}*/ // FIXME: until 5.0.0 we need to suppress warnings, and Scala 3 is being annoying (refreshing change)
136-
type Ref_ = Ref[_]
137-
refs.zip(freeSlots).foreach { (refAndAddr: (Ref[_], Int) @nowarn3) =>
138-
val ref: Ref_ @nowarn3 = refAndAddr._1
139-
val addr = refAndAddr._2
140-
ref.allocate(addr)
141-
allocatedSlots += addr
142-
}
143-
allocatedSlots.toList
144-
}
145-
146102
/** If required, generates callee-save around a main body of instructions.
147103
*
148104
* This is needed when using `flatMap`, as it is unaware of the register
@@ -160,23 +116,19 @@ private [deepembedding] object StrictParsley {
160116
* @param instrs the instruction buffer
161117
* @param state the code generation state, for label generation
162118
*/
163-
private def generateCalleeSave[M[_, +_]: ContOps, R](numRegsUsedByParent: Int, bodyGen: =>M[R, Unit], usedRefs: Set[Ref[_]])
164-
(implicit instrs: InstrBuffer, state: CodeGenState): M[R, Unit] = {
165-
val reqRegs = usedRefs.size
166-
val localRegs: Set[Ref[_]] @nowarn3 = usedRefs.filterNot(_.allocated): @nowarn3
167-
val allocatedRegs = allocateRegisters(localRegs, usedRefs)
168-
val calleeSaveRequired = numRegsUsedByParent >= 0 // if this is -1, then we are the top level and have no parent, otherwise it needs to be done
169-
if (calleeSaveRequired && localRegs.nonEmpty) {
170-
val end = state.freshLabel()
171-
val calleeSave = state.freshLabel()
172-
instrs += new instructions.Label(calleeSave)
173-
instrs += new instructions.CalleeSave(end, localRegs, reqRegs, allocatedRegs, numRegsUsedByParent)
174-
bodyGen |> {
175-
instrs += new instructions.Jump(calleeSave)
176-
instrs += new instructions.Label(end)
119+
private def allocateAndExpandRefs(minRef: Int, usedRefs: Set[Ref[_]])(implicit instrs: InstrBuffer): Unit = {
120+
var nextSlot = math.max(minRef, 0)
121+
usedRefs.foreach { (r: Ref[_]) =>
122+
if (!r.allocated) {
123+
r.allocate(nextSlot)
124+
nextSlot += 1
177125
}
178126
}
179-
else bodyGen
127+
val totalSlotsRequired = nextSlot
128+
// if this is -1, then we are the top level and have no parent, otherwise it needs to be done
129+
if (minRef >= 0 && (minRef < totalSlotsRequired)) {
130+
instrs += new instructions.ExpandRefs(totalSlotsRequired)
131+
}
180132
}
181133

182134
/** Generates each of the shared, non-recursive, parsers that have been ''used'' by

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

+3-8
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
4444
// The instructions used to execute this parser along with the number of registers it uses
4545
final private [parsley] lazy val (instrs: Array[Instr], numRegs: Int) = computeInstrs
4646

47-
/** This parser is the result of a `flatMap` operation, and as such must perform
48-
* callee-save on `numRegs` registers (which belong to its parent)
49-
*
50-
* @param numRegs the number of registers the parent uses (these must be saved)
51-
*/
52-
private [deepembedding] def demandCalleeSave(numRegs: Int): Unit = numRegsUsedByParent = numRegs
47+
private [deepembedding] def setMinReferenceAllocation(minRef: Int): Unit = this.minRef = minRef
5348

5449
// Internals
5550
// To ensure that stack-overflow cannot occur during the processing of particularly
@@ -87,7 +82,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
8782
final private var cps = false
8883
final private [deepembedding] def isCps: Boolean = cps
8984
/** how many registers are used by the ''parent'' of this combinator (this combinator is part of a `flatMap` when this is not -1) */
90-
final private var numRegsUsedByParent = -1
85+
final private var minRef = -1
9186

9287
/** Computes the instructions associated with this parser as well as the number of
9388
* registers it requires in a (possibly) stack-safe way.
@@ -120,7 +115,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
120115
implicit val letMap: LetMap = LetMap(letFinderState.lets, letFinderState.recs)
121116
for { sp <- this.optimised } yield {
122117
implicit val state: backend.CodeGenState = new backend.CodeGenState(letFinderState.numRegs)
123-
sp.generateInstructions(numRegsUsedByParent, usedRefs, letMap.bodies)
118+
sp.generateInstructions(minRef, usedRefs, letMap.bodies)
124119
}
125120
}
126121
}, letFinderState.numRegs)

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,17 @@ private [internal] object Apply extends Instr {
7272
}
7373

7474
// Monadic
75-
private [internal] final class DynCall(f: Any => Array[Instr]) extends Instr {
75+
private [internal] final class DynCall(f: (Any, Int) => Array[Instr]) extends Instr {
7676
override def apply(ctx: Context): Unit = {
7777
ensureRegularInstruction(ctx)
78-
ctx.call(f(ctx.stack.upop()))
78+
ctx.call(f(ctx.stack.upop(), ctx.regs.size))
7979
}
8080
// $COVERAGE-OFF$
8181
override def toString: String = "DynCall(?)"
8282
// $COVERAGE-ON$
8383
}
8484
private [internal] object DynCall {
85-
def apply[A](f: A => Array[Instr]): DynCall = new DynCall(f.asInstanceOf[Any => Array[Instr]])
85+
def apply[A](f: (A, Int) => Array[Instr]): DynCall = new DynCall(f.asInstanceOf[(Any, Int) => Array[Instr]])
8686
}
8787

8888
// Control Flow

0 commit comments

Comments
 (0)