Compile functions with $-prefixed parameters instead of bailing

include/openscad_cpp_evaluator/bytecode.hpp

@@ -241,7 +241,11 @@ struct Instruction {
 struct CompiledChunk {
     struct Param {
         std::string name;
-        int slot = 0;
+        int slot = 0; // unused when isDyn is true -- $-params are never slot-addressed
+        // $-prefixed parameter: binds through ctx.dyn (dynamically scoped)
+        // instead of a slot -- see compileFunctionLike's own doc comment,
+        // bytecode_compiler.cpp, and bindCompiledArgs', bytecode_vm.cpp.
+        bool isDyn = false;
     };
 
     // A call site statically resolved at compile time (see

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build"
 
 [project]
 name = "openscad_cpp_evaluator"
-version = "0.9.2"
+version = "0.10.0"
 description = "C++ OpenSCAD evaluator with Python bindings"
 readme = "README.md"
 requires-python = ">=3.12"

src/bytecode_compiler.cpp

@@ -193,7 +193,7 @@ class Compiler;
 // (a non-empty `enclosing` chain, letting the literal's own free-variable
 // references resolve as upvalues into whatever lexically contains it).
 // Returns false (chunk left partially populated, discarded by the caller)
-// for a $-prefixed parameter or any NotCompilable thrown while compiling.
+// for any NotCompilable thrown while compiling.
 bool compileFunctionLike(CompiledChunk& chunk, const oscad::Scope* staticScope, const oscad::ASTNode* selfDecl,
                           std::vector<EnclosingLevel> enclosing,
                           const std::vector<std::unique_ptr<oscad::ParameterDeclaration>>& params,
@@ -1075,21 +1075,21 @@ bool compileFunctionLike(CompiledChunk& chunk, const oscad::Scope* staticScope,
                           std::vector<EnclosingLevel> enclosing,
                           const std::vector<std::unique_ptr<oscad::ParameterDeclaration>>& params,
                           const oscad::Expression& bodyExpr) {
-    for (const auto& p : params) {
-        // $-prefixed parameters bind through ctx.dyn (dynamically scoped,
-        // never slot-addressed) via the existing applyDefaults/bindArgs
-        // machinery -- out of scope for this design, see this file's
-        // header comment.
-        if (!p->name->name.empty() && p->name->name[0] == '$') return false;
-    }
-
     chunk.selfDecl = selfDecl;
     Compiler compiler(chunk, staticScope, selfDecl, std::move(enclosing));
     CompileScope bodyScope;
     bodyScope.push();
     for (const auto& p : params) {
-        int slot = compiler.declareLocal(bodyScope, p->name->name);
-        chunk.params.push_back(CompiledChunk::Param{p->name->name, slot});
+        // $-prefixed parameters bind through ctx.dyn (dynamically scoped)
+        // rather than a slot -- never declared as a local, so a reference
+        // to one inside the body already compiles to Op::LoadDyn via the
+        // Identifier case's own $-check regardless of scope visibility.
+        // Still occupies its declared POSITION among all parameters (see
+        // bindCompiledArgs, bytecode_vm.cpp) for positional-argument
+        // matching, exactly like the interpreter's own bindArgs.
+        const bool isDyn = !p->name->name.empty() && p->name->name[0] == '$';
+        const int slot = isDyn ? 0 : compiler.declareLocal(bodyScope, p->name->name);
+        chunk.params.push_back(CompiledChunk::Param{p->name->name, slot, isDyn});
     }
     chunk.defaultCode.resize(params.size());
 

src/bytecode_vm.cpp

@@ -604,17 +604,18 @@ Value runChunk(Evaluator& ev, const CompiledChunk& chunk, const std::vector<Inst
 // Mirrors Evaluator::bindArgs' own positional/named matching exactly (same
 // "later write for a repeated name wins," same "evaluate every argument
 // expression even if its name doesn't match a declared parameter" side-
-// effect rule), but writes matched-plain-parameter values directly into
-// `slots` instead of a fresh unordered_map, and routes a matched-$-name
-// straight to `childCtx.dyn` (childCtx.dyn->set) since $-parameters never
-// reach compilation (see tryCompileFunction) -- so ANY $-named argument here
-// is necessarily an undeclared override, exactly like today's interpreter
-// path. An undeclared plain (non-$) name has no slot and nothing in this
-// chunk can ever reference it by name, so its already-evaluated value is
-// simply discarded -- ponytail: ctx.let_ isn't written for it either
-// (unlike the interpreter path), which changes nothing observable, since
-// nothing downstream of a compiled call ever reads ctx.let_ for a name a
-// compiled body didn't itself resolve to a slot.
+// effect rule), but writes a matched plain-parameter value directly into
+// `slots` instead of a fresh unordered_map, and routes a matched DECLARED
+// $-parameter (chunk.params[i].isDyn) straight to `childCtx.dyn` instead of
+// a slot -- see compileFunctionLike's own doc comment, bytecode_compiler.cpp.
+// A $-named argument that doesn't match any declared parameter is still an
+// undeclared override, exactly like today's interpreter path -- routed to
+// `childCtx.dyn` too. An undeclared plain (non-$) name has no slot and
+// nothing in this chunk can ever reference it by name, so its already-
+// evaluated value is simply discarded -- ponytail: ctx.let_ isn't written
+// for it either (unlike the interpreter path), which changes nothing
+// observable, since nothing downstream of a compiled call ever reads
+// ctx.let_ for a name a compiled body didn't itself resolve to a slot.
 void bindCompiledArgs(Evaluator& ev, const CompiledChunk& chunk,
                        const std::vector<std::unique_ptr<oscad::Argument>>& arguments, EvalContext& callerCtx,
                        EvalContext& childCtx, std::vector<Value>& slots, std::vector<bool>& bound) {
@@ -628,7 +629,11 @@ void bindCompiledArgs(Evaluator& ev, const CompiledChunk& chunk,
             bool matched = false;
             for (size_t i = 0; i < nparams; ++i) {
                 if (chunk.params[i].name == name) {
-                    slots[static_cast<size_t>(chunk.params[i].slot)] = std::move(v);
+                    if (chunk.params[i].isDyn) {
+                        childCtx.dyn->set(name, std::move(v));
+                    } else {
+                        slots[static_cast<size_t>(chunk.params[i].slot)] = std::move(v);
+                    }
                     bound[i] = true;
                     matched = true;
                     break;
@@ -641,14 +646,41 @@ void bindCompiledArgs(Evaluator& ev, const CompiledChunk& chunk,
             auto& a = static_cast<const oscad::PositionalArgument&>(*argPtr);
             Value v = ev.evalExpr(*a.expr, callerCtx);
             if (positionalIdx < nparams) {
-                slots[static_cast<size_t>(chunk.params[positionalIdx].slot)] = std::move(v);
+                const CompiledChunk::Param& p = chunk.params[positionalIdx];
+                if (p.isDyn) {
+                    childCtx.dyn->set(p.name, std::move(v));
+                } else {
+                    slots[static_cast<size_t>(p.slot)] = std::move(v);
+                }
                 bound[positionalIdx] = true;
             }
             ++positionalIdx;
         }
     }
 }
 
+// Shared by runCompiledFunction/runCompiledFunctionFromBound: applies each
+// unbound parameter's default (or, for a $-parameter with none, an explicit
+// undef -- mirrors Evaluator::applyDefaults' own "declaring a parameter
+// always creates a fresh binding" rule, which for a slot-addressed plain
+// parameter is already true for free via frame.slots' own zero-initialized
+// Value{} fill, but for ctx.dyn needs the same explicit write applyDefaults
+// makes, or a stale ancestor-level $-var would incorrectly show through).
+void applyCompiledDefaults(Evaluator& ev, const CompiledChunk& chunk, VmFrame& frame, EvalContext& childCtx) {
+    for (size_t i = 0; i < chunk.params.size(); ++i) {
+        if (frame.bound[i]) continue;
+        const CompiledChunk::Param& p = chunk.params[i];
+        if (p.isDyn) {
+            childCtx.dyn->set(p.name, chunk.defaultCode[i].empty()
+                                           ? Value{}
+                                           : runChunk(ev, chunk, chunk.defaultCode[i], frame.slots, childCtx, frame.stack));
+        } else if (!chunk.defaultCode[i].empty()) {
+            frame.slots[static_cast<size_t>(p.slot)] =
+                runChunk(ev, chunk, chunk.defaultCode[i], frame.slots, childCtx, frame.stack);
+        }
+    }
+}
+
 } // namespace
 
 Value runCompiledFunction(Evaluator& ev, const CompiledChunk& chunk,
@@ -667,12 +699,7 @@ Value runCompiledFunction(Evaluator& ev, const CompiledChunk& chunk,
     // started running, never during argument binding itself.
     ev.setCurrentCallVmFrame(&frame);
     bindCompiledArgs(ev, chunk, arguments, callerCtx, childCtx, frame.slots, frame.bound);
-    for (size_t i = 0; i < chunk.params.size(); ++i) {
-        if (!frame.bound[i] && !chunk.defaultCode[i].empty()) {
-            frame.slots[static_cast<size_t>(chunk.params[i].slot)] =
-                runChunk(ev, chunk, chunk.defaultCode[i], frame.slots, childCtx, frame.stack);
-        }
-    }
+    applyCompiledDefaults(ev, chunk, frame, childCtx);
     return runChunk(ev, chunk, chunk.bodyCode, frame.slots, childCtx, frame.stack, tailOut);
 }
 
@@ -684,8 +711,13 @@ Value runCompiledFunctionFromBound(Evaluator& ev, const CompiledChunk& chunk, co
     frame.bound.assign(chunk.params.size(), false);
     ev.setCurrentCallVmFrame(&frame);
     for (size_t i = 0; i < chunk.params.size(); ++i) {
-        if (const Value* v = bound.find(chunk.params[i].name)) {
-            frame.slots[static_cast<size_t>(chunk.params[i].slot)] = *v;
+        const CompiledChunk::Param& p = chunk.params[i];
+        if (const Value* v = bound.find(p.name)) {
+            if (p.isDyn) {
+                childCtx.dyn->set(p.name, *v);
+            } else {
+                frame.slots[static_cast<size_t>(p.slot)] = *v;
+            }
             frame.bound[i] = true;
         }
     }
@@ -699,12 +731,7 @@ Value runCompiledFunctionFromBound(Evaluator& ev, const CompiledChunk& chunk, co
         }
         if (!matched && !k.empty() && k[0] == '$') childCtx.dyn->set(k, v);
     }
-    for (size_t i = 0; i < chunk.params.size(); ++i) {
-        if (!frame.bound[i] && !chunk.defaultCode[i].empty()) {
-            frame.slots[static_cast<size_t>(chunk.params[i].slot)] =
-                runChunk(ev, chunk, chunk.defaultCode[i], frame.slots, childCtx, frame.stack);
-        }
-    }
+    applyCompiledDefaults(ev, chunk, frame, childCtx);
     return runChunk(ev, chunk, chunk.bodyCode, frame.slots, childCtx, frame.stack, tailOut);
 }
 

tests/test_bytecode_compiler.cpp

@@ -175,10 +175,34 @@ TEST(BytecodeCompiler, NonCompilableFunctionContainingACallStillFallsBackCorrect
               "ECHO: 8");
 }
 
-TEST(BytecodeCompiler, DollarPrefixedParameterBailsCompilationAndStillWorks) {
-    ScopedVm vm(true);
+TEST(BytecodeCompiler, DollarPrefixedParameterCompiles) {
+    ScopedVm vm(true);
+    // $-prefixed parameters used to bail compileFunctionLike outright
+    // (bound through ctx.dyn, never slot-addressed) -- now they compile:
+    // never declared as a local (skipped in the params/declareLocal loop),
+    // so every reference inside the body still reaches Op::LoadDyn via the
+    // Identifier case's own $-check, exactly as an undeclared override
+    // already did.
     EXPECT_EQ(runCapturingEcho("function withFn($fn = 8) = $fn;\necho(withFn());"), "ECHO: 8");
     EXPECT_EQ(runCapturingEcho("function withFn($fn = 8) = $fn;\necho(withFn($fn = 20));"), "ECHO: 20");
+    // Positional binding: a $-param still occupies its declared POSITION
+    // among all parameters for a caller's positional arguments, exactly
+    // like the interpreter's own bindArgs.
+    EXPECT_EQ(runCapturingEcho("function withFn($fn, x) = $fn + x;\necho(withFn(10, 1));"), "ECHO: 11");
+    // Declared without a default and not passed: shadows to undef locally
+    // rather than inheriting an ancestor's dyn binding -- mirrors
+    // Evaluator::applyDefaults' explicit "declaring a parameter always
+    // creates a fresh binding" write.
+    EXPECT_EQ(runCapturingEcho("function withFn($fn) = $fn;\necho(let($fn = 99) withFn());"), "ECHO: undef");
+    // The proof this is actually running compiled, not silently falling
+    // back: same ternary-bodied shape and technique as
+    // FastContinueWithNoBreakpointInFunctionUsesVm above (3 stops =
+    // compiled, 5 = interpreted), with $fn substituted in for the plain
+    // parameter both as the declared name and every body reference.
+    const int stops = countDebugHookStops("function f($fn) = $fn > 0 ? $fn + 1 : $fn - 1;\n"
+                                           "echo(f(5));",
+                                           std::unordered_map<std::string, std::set<int>>{{"<string>", {2}}});
+    EXPECT_EQ(stops, 3);
 }
 
 TEST(BytecodeCompiler, UndeclaredDollarNamedArgumentReachesDynInsideCompiledFunction) {
@@ -735,27 +759,26 @@ TEST(BytecodeCompiler, TernaryWrappedMutualRecursionResolvesCorrectly) {
     EXPECT_EQ(runCapturingEcho(script), "ECHO: true\nECHO: false");
 }
 
-TEST(BytecodeCompiler, ClosureWithDollarParameterStillBailsContainer) {
-    ScopedVm vm(true);
-    // Residual, deliberate limitation: a FunctionLiteral with its OWN
-    // dollar-prefixed parameter still fails compileFunctionLike outright
-    // (same $-prefixed-parameter rule as a plain named function, see
-    // DollarPrefixedParameterBailsCompilationAndStillWorks above) -- there's
-    // no lighter way left to discover what it captures in that case, so the
-    // WHOLE containing declaration still bails (throw NotCompilable), same
-    // as before Op::MakeClosure. Value must still be correct via the
-    // interpreter fallback.
-    const int stops = countDebugHookStops(
-        "function outer(x) = let(g = function(y, $fn) y + x + $fn) g(5, $fn=2);\n"
-        "echo(outer(10));",
-        std::nullopt);
-    // `outer` never compiles at all here (more than 1 stop for its own
-    // call), unlike MakeClosureLetsContainerCompileDespiteNonEscapingClosure
-    // above where the analogous ($-free) container compiles to 1.
-    EXPECT_GT(stops, 2);
-    EXPECT_EQ(runCapturingEcho("function outer(x) = let(g = function(y, $fn) y + x + $fn) g(5, $fn=2);\n"
-                                "echo(outer(10));"),
-              "ECHO: 17");
+TEST(BytecodeCompiler, ClosureWithDollarParameterNowCompilesToo) {
+    ScopedVm vm(true);
+    // A FunctionLiteral with its OWN dollar-prefixed parameter used to fail
+    // compileFunctionLike outright, bailing the WHOLE containing
+    // declaration (see DollarPrefixedParameterCompiles above for the plain-
+    // function case) -- now compileFunctionLike has no $-specific bail left
+    // at all, so this compiles too, container and closure both.
+    const std::string script = "function outer(x) = let(g = function(y, $fn) y + x + $fn) g(5, $fn=2);\n"
+                                "echo(outer(10));";
+    EXPECT_EQ(runCapturingEcho(script), "ECHO: 17");
+    // Proof via the same fast-continue stop-count technique used
+    // throughout this file (nullopt is NOT a reliable signal here -- it
+    // forces every function to interpret regardless of eligibility, see
+    // DebugAttachedWithoutFastContinueAlwaysInterprets above -- a real
+    // breakpoint map on a non-matching line is what actually distinguishes
+    // compiled from interpreted). 3 stops: outer's own body-entry, g's
+    // call-site, g's own body-entry -- no ternary/sub-expression stops for
+    // either, since both now run compiled.
+    const int stops = countDebugHookStops(script, std::unordered_map<std::string, std::set<int>>{{"<string>", {2}}});
+    EXPECT_EQ(stops, 3);
 }
 
 // -- Tail-call optimization, VM path (Phase B) -----------------------------

tests/test_tail_calls.cpp

@@ -119,6 +119,21 @@ TEST(TailCalls, DollarVarSetEarlyInATailChainStaysVisibleManyHopsLater) {
     EXPECT_EQ(runCapturingEcho(script), "ECHO: 77");
 }
 
+TEST(TailCalls, DollarParameterThreadsThroughCompiledTailRecursion) {
+    ScopedVm vm(true);
+    // $fn declared as its own PARAMETER (not just read as an ambient dyn
+    // var, unlike the test above) rebinds via childCtx.dyn at every tail
+    // hop -- compileFunctionLike no longer bails on a $-prefixed parameter
+    // (see its own doc comment, bytecode_compiler.cpp). No tail-call-
+    // specific hazard: every hop's bindCompiledArgs call is identical to
+    // what a genuine non-tail call already does, and dyn already threads
+    // correctly across hops (proved by the test above).
+    const std::string script =
+        "function sum_with_fn($fn, n, acc=0) = n == 0 ? $fn + acc : sum_with_fn($fn, n - 1, acc + n);\n"
+        "echo(sum_with_fn(1000, 50000));";
+    EXPECT_EQ(runCapturingEcho(script), "ECHO: 1.25003e+9");
+}
+
 TEST(TailCalls, NonTailRecursionIsUnaffectedByTheTrampoline) {
     ScopedVm vm(false);
     // `n * fact(n-1)` is NOT a tail position -- the multiply happens after
@@ -316,6 +331,32 @@ TEST(TailCalls, DeepNonTailRecursionHitsAControlledErrorInsteadOfCrashingCompile
     EXPECT_THROW(ev.resolveTree(ast, ctx), EvalError);
 }
 
+TEST(TailCalls, CollatzStepsRecursesFromBothTernaryBranchesInterpreted) {
+    ScopedVm vm(false);
+    // Real-world example exercising tail calls from BOTH ternary branches at
+    // once (the even and odd cases each recurse) -- came up when auditing
+    // for un-trampolined tail positions (see this file's header). Scans
+    // every n < 10000 and takes the longest chain; the known record holder
+    // in that range is n=6171 at 261 steps, calling collatz_steps 10000
+    // times over.
+    const std::string script =
+        "function collatz_steps(n, steps=0) = n == 1 ? steps : "
+        "(n % 2 == 0 ? collatz_steps(n / 2, steps + 1) : collatz_steps(3 * n + 1, steps + 1));\n"
+        "lengths = [for (i = [1:10000]) collatz_steps(i)];\n"
+        "echo(max(lengths));";
+    EXPECT_EQ(runCapturingEcho(script), "ECHO: 261");
+}
+
+TEST(TailCalls, CollatzStepsRecursesFromBothTernaryBranchesCompiled) {
+    ScopedVm vm(true);
+    const std::string script =
+        "function collatz_steps(n, steps=0) = n == 1 ? steps : "
+        "(n % 2 == 0 ? collatz_steps(n / 2, steps + 1) : collatz_steps(3 * n + 1, steps + 1));\n"
+        "lengths = [for (i = [1:10000]) collatz_steps(i)];\n"
+        "echo(max(lengths));";
+    EXPECT_EQ(runCapturingEcho(script), "ECHO: 261");
+}
+
 TEST(TailCalls, ShallowNonTailRecursionStillWorksUnderTheDepthGuard) {
     ScopedVm vm(true);
     // The guard must not fire for perfectly ordinary, shallow non-tail