chore: added notes about why DynCall cannot suppress results

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

@@ -94,12 +94,13 @@ 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 =>
-                val p = f(x)
-                // FIXME: suppress results within p, then can remove pop
-                p.demandCalleeSave(state.numRegs)
-                if (implicitly[ContOps[M]].isStackSafe) p.overflows()
-                p.instrs
+                val q = f(x)
+                q.demandCalleeSave(state.numRegs)
+                if (implicitly[ContOps[M]].isStackSafe) q.overflows()
+                q.instrs
             }
+            // NOTE: this cannot be removed, because `q`'s instructions are cached: there is no way to tell it
+            // to not produce results that doesn't stop it working later. Something like `p.flatten ~> p.flatten` will crash.
             if (!producesResults) instrs += instructions.Pop
         }
     }

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

@@ -161,11 +161,11 @@ private [deepembedding] object StrictParsley {
         if (calleeSaveRequired && localRegs.nonEmpty) {
             val end = state.freshLabel()
             val calleeSave = state.freshLabel()
-            instrs += new instructions.Push(false)
+            instrs += new instructions.Push(false) // callee-save is not active
             instrs += new instructions.Label(calleeSave)
             instrs += new instructions.CalleeSave(end, localRegs, reqRegs, allocatedRegs, numRegsUsedByParent)
             bodyGen |> {
-                instrs += new instructions.Push(true)
+                instrs += new instructions.Push(true) // callee-save is active
                 instrs += new instructions.Jump(calleeSave)
                 instrs += new instructions.Label(end)
             }

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

@@ -156,12 +156,15 @@ private [internal] object Span extends Instr {
 
 // FIXME: this isn't re-entrant, the stack allows us to tell, but the persisted
 // references are shared across nested visits. Is the reference deallocation idea also screwy?
-// perhaps deallocation only occurs when we have an empty restore stack?
+// perhaps deallocation only occurs when we have an empty restore stack (I think this is true,
+// because deallocation allows for the reference to be remapped later, but we always want the
+// same mapping for a let-bound parser)
+// TODO: unit test to demonstrate the above issue!
 // This instruction holds mutable state, but it is safe to do so, because it's always the first instruction of a DynCall.
-private [parsley] final class CalleeSave(var label: Int, localRegs: Set[Ref[_]], reqSize: Int, slots: List[(Int, Int)], saveArray: Array[AnyRef])
+private [parsley] final class CalleeSave(var label: Int, localRefs: Set[Ref[_]], reqSize: Int, slots: List[(Int, Int)], saveArray: Array[AnyRef])
     extends InstrWithLabel {
-    private def this(label: Int, localRegs: Set[Ref[_]], reqSize: Int, slots: List[Int]) =
-        this(label, localRegs, reqSize, slots.zipWithIndex, new Array[AnyRef](slots.length))
+    private def this(label: Int, localRefs: Set[Ref[_]], reqSize: Int, slots: List[Int]) =
+        this(label, localRefs, reqSize, slots.zipWithIndex, new Array[AnyRef](slots.length))
     // this filters out the slots to ensure we only do callee-save on registers that might exist in the parent
     def this(label: Int, localRefs: Set[Ref[_]], reqSize: Int, slots: List[Int], numRegsInContext: Int) =
         this(label, localRefs, reqSize, slots.takeWhile(_ < numRegsInContext))
@@ -191,7 +194,7 @@ private [parsley] final class CalleeSave(var label: Int, localRegs: Set[Ref[_]],
         }
         // This is the only way to get them reallocated on the next invocation
         // FIXME: I think this isn't thread-safe, because two flatMaps can simulataneously reallocate?
-        localRegs.foreach(_.deallocate())
+        localRefs.foreach(_.deallocate())
     }
 
     private def continue(ctx: Context): Unit = {