From 20b19230e65eb54e5660b662b35a1d61a6ef4b89 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 03:25:53 -0700 Subject: [PATCH 1/5] Rewrite the bytecode VM onto an explicit heap frame stack Native C++ recursion between compiled chunks (function calls, and now module calls too) is replaced with an explicit, heap-allocated call stack (Evaluator::vmCallStack_) serviced by one driver loop (driveVm), so a compiled-to-compiled recursive chain no longer grows the native stack at all -- bounded instead by a new kMaxVmCallStackDepth (1,000,000), not the native-stack-safety kMaxUserCallDepth (50) that used to cap every recursive call regardless of shape. - Functions: Op::CallFn/CallFnTail/CallDynamic/CallDynamicTail push/ replace explicit VmFrames instead of recursing natively or through the old runCompiledFunctionTrampoline (retired, along with TailCallRequest). - Modules: bodies compile too (Op::CallModule, plus NativeStatement/ NativeCondJumpIfFalse/NativeIterMaterialize/ForIterNext/ForIterEnd for control flow) -- no trampoline ever existed for modules, so a recursive tree/pattern-generator module used to segfault around depth ~5000 on an ordinary build. if/for get real Jump-based bytecode (so a recursive module call nested inside one doesn't hide behind a native call boundary); everything else (assignment, echo, assert, let-blocks, modifiers, intersection_for, a builtin/ unresolved module call) stays a native passthrough for one statement at a time -- those are leaf-shaped, never the recursion risk this targets. - Fixed an O(depth^2) perf cliff this surfaced: Evaluator::callCtxFor's closure/lexical-nesting detection used to scan the full callStack_ on every call. Replaced with activeDeclRefcount_, a refcounted set of DISTINCT active declarations (a deep call chain is overwhelmingly the same declaration repeated, not many different ones) -- ~50x faster at depth 40,000, and scaling is linear again. 695 tests green under both OSCAD_BYTECODE_VM on and off. Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/bytecode.hpp | 159 +- .../bytecode_compiler.hpp | 92 +- .../openscad_cpp_evaluator/bytecode_vm.hpp | 291 ++- include/openscad_cpp_evaluator/evaluator.hpp | 417 +++- pyproject.toml | 2 +- src/builtins/call_args.cpp | 4 +- src/bytecode_compiler.cpp | 297 ++- src/bytecode_vm.cpp | 1684 ++++++++++------- src/csg_resolve.cpp | 74 +- src/stmt_eval.cpp | 23 +- src/user_calls.cpp | 213 ++- tests/test_bytecode_compiler.cpp | 307 ++- tests/test_control_flow.cpp | 32 + tests/test_debug_hooks.cpp | 14 +- tests/test_tail_calls.cpp | 69 +- 15 files changed, 2701 insertions(+), 977 deletions(-) diff --git a/include/openscad_cpp_evaluator/bytecode.hpp b/include/openscad_cpp_evaluator/bytecode.hpp index b8c89d4..d58ffbe 100644 --- a/include/openscad_cpp_evaluator/bytecode.hpp +++ b/include/openscad_cpp_evaluator/bytecode.hpp @@ -12,6 +12,9 @@ namespace oscad { class FunctionDeclaration; class FunctionLiteral; +class ModuleDeclaration; +class ModularCall; +class Expression; } // namespace oscad namespace oscadeval { @@ -37,7 +40,33 @@ enum class Op { StoreLocal, // a = slot index (pops) LoadDyn, // a = name pool index ($-prefixed) StoreDyn, // a = name pool index (pops; also marks dynExplicit) - LoadFree, // a = name pool index -- Evaluator::evalIdentifier fallback + LoadFree, // a = name pool index -- Evaluator::evalIdentifier fallback (warnIfUndef=true) + // Same as LoadFree, but Evaluator::evalIdentifier's warnIfUndef=false -- + // only ever emitted for a PrimaryCall's own callee probe (bytecode_ + // compiler.cpp's PrimaryCall case), mirroring evalFunctionCall's own + // `evalIdentifier(leftId->name, ..., /*warnIfUndef=*/false)` fallback + // exactly: probing whether a bare identifier resolves to a callable + // Closure value shouldn't itself warn "unknown variable" when it + // doesn't -- Op::CallDynamic's own runtime handler already warns + // "unknown function" once the probe comes back non-Closure, so using + // plain LoadFree here doubled that up into two warnings for one + // genuinely-unknown callee. + LoadFreeNoWarn, // a = name pool index + // a = slot index, b = name pool index (pops). Only emitted for a + // compiled ASSIGNMENT BLOCK (Evaluator::tryCompileAssignmentBlock, + // user_calls.cpp -- a run of sibling `name = expr;` statements sharing + // one scope, exactly what Evaluator::evalChildren already isolates as + // its own `assignments` sub-list before any non-assignment statement + // runs). Stores into BOTH the slot (so a LATER sibling assignment in + // the SAME batch can read this one back via a fast LoadLocal, mirroring + // how LetOp's own bindings already work) AND ctx.let_ (so code OUTSIDE + // this compiled batch -- a later non-assignment statement, a nested + // block, a module call -- still sees it the ordinary way, since this + // batch's own slots are invisible once it returns). Plain StoreLocal + // alone would lose that outer visibility entirely; plain "store to + // let_ only" would lose the fast intra-batch slot reads a batch of + // assignments exists to buy back in the first place. + StoreLocalAndLet, Range, // pops step, pops end, pops start (pushed in that order); pushes applyRange(start, end, step) Index, // pops idx, pops obj, pushes applyIndexAccess(obj, idx) Member, // a = name pool index (member name); pops obj, pushes applyMemberAccess(obj, name) @@ -210,6 +239,101 @@ enum class Op { // which recomputes this on every failing call). pos/node: same // Evaluator::error() TRACE-walk need CheckIterLimit already has. AssertFail, + + // -- Module-body compilation (Stage 2) -------------------------------- + // A call to a user module resolved to a specific ModuleDeclaration AT + // COMPILE TIME (see CompiledChunk::ModuleCallSite) -- the module-side + // analog of Op::CallFn. a = index into CompiledChunk::moduleCallSites. + // Arguments are NOT pre-pushed onto the operand stack the way CallFn's + // are (bindArgs runs directly against site.callNode->arguments at + // runtime instead) -- module-call argument evaluation was never the + // recursion-safety target here, so there's no need to teach the + // compiler a separate "push each arg" step just for this. Produces no + // stack Value at all: its effect is entirely the side effect of + // splicing (or wrapping into a synthetic "union") whatever CSGNodes + // the callee's body produces into the CURRENT treeStack_ frame -- see + // Evaluator::spliceModuleChildren's own doc comment (bytecode_vm.cpp) + // for why this is always the splice branch, never the "wrap as a + // builtin-tagged CSGNode" one evalModularCall's own general form also + // has (Op::CallModule only ever targets a resolved USER module, for + // which splice is unconditional). + CallModule, + + // A single "native passthrough" statement -- assignment, echo, assert, + // let-block, modifier wrapper, intersection_for, children(), a builtin + // module call, or a user-module call that didn't resolve at compile + // time (shadowed, forward-declared, or otherwise not staticaly known) + // -- anything compileStatementList doesn't give its own real bytecode. + // a = index into CompiledChunk::nativeStatements. Runtime just does + // what Evaluator::evalChildren's own per-statement loop already does + // for one node: derive childCtx via ctx.withScope(...), checkDebug + // (skipped for ModularLet, exactly like evalChildren's own condition + // -- ModularLet fires its own per-assignment checkDebug internally), + // evalStatement. These are "leaf-shaped" in the sense the Stage 2 plan + // means it: whatever native recursion they make (a builtin's own + // resolve function walking ITS children, evalLetBlock, etc.) is + // bounded by ordinary script structure, not by how deep a recursive + // module chain goes -- that risk is fully covered by CallModule/ + // ForIterNext/the Jump-based if/for control flow instead, regardless + // of how many of THESE sit alongside them in the same body. + NativeStatement, + + // If/if-else's own condition, evaluated NATIVELY (Evaluator:: + // evalExprMaybeCompiled -- gets the SAME expression-level compilation + // a bare statement-context expression already would, just not folded + // into THIS chunk's own instruction stream) rather than compiled + // inline -- module-body compilation's job is the STATEMENT-level + // control flow (so a recursive module call nested inside doesn't hide + // behind a native evalStatement call), not re-deriving expression + // compilation tryCompileStatementExpr already does. a = index into + // CompiledChunk::nativeExprs, b = jump target on false (mirrors + // JumpIfFalse's own semantics, just with a native condition source + // instead of a popped stack Value). + NativeCondJumpIfFalse, + + // The "entering this branch/iteration" expr-level checkDebug marker + // ModularIf/ModularIfElse/ModularFor's own interpreted forms each fire + // once (see evalStatement's ModularIf/ModularIfElse cases and evalFor, + // stmt_eval.cpp) -- a = index into CompiledChunk::nativeStatements + // (reusing that same table; the entry here is whichever ASTNode the + // interpreter would have passed to checkDebug -- the branch/body's + // own first statement, or the ModularIf/For node itself when that + // branch/body is empty). + NativeCheckDebugExprLevel, + + // One ModularFor assignment's own RHS range/list expression, evaluated + // NATIVELY (same reasoning as NativeCondJumpIfFalse) and materialized + // into an IterList exactly like the existing (list-comprehension) + // Op::IterMaterialize -- a = index into CompiledChunk::nativeExprs, b + // = iterList id. + NativeIterMaterialize, + + // Statement-for's own per-iteration advance -- the module-body analog + // of Op::IterNext, but binding into ctx.let_ (a real, dynamically- + // discoverable statement-scope variable, exactly like the + // interpreter's own per-iteration childCtx.let_->set()) instead of a + // slot, and -- critically -- pushing a FRESH child EvalContext onto + // f.ctxChain for the upcoming iteration rather than mutating the + // current one in place: evalFor derives a brand-new childCtx per + // iteration (see stmt_eval.cpp), so an ordinary local assignment made + // inside the loop body doesn't trip evalAssignment's own "was + // assigned on line N but overwritten" warning on iteration 2 onward, + // and so a value bound in one iteration never leaks into the next. + // a = name-pool index (the loop variable's own name), b = iterList id, + // c = jump target on exhaustion (taken WITHOUT pushing a new ctx -- + // mirrors the cartesian loop simply not entering the body once that + // dimension is exhausted). `node` = the Assignment AST node, for the + // matching per-iteration checkDebug call (mirrors evalFor's own + // `checkDebug(*node.assignments[depth], childCtx)`). Always paired + // with a matching Op::ForIterEnd at the bottom of the SAME dimension's + // loop body. + ForIterNext, + + // Pops the ctx Op::ForIterNext just pushed for the iteration that's + // ending (f.ctxChain.pop_back()) and jumps back to that same + // ForIterNext instruction to attempt the next one. a = jump target + // (the matching ForIterNext's own pc). + ForIterEnd, }; struct Instruction { @@ -339,6 +463,33 @@ struct CompiledChunk { std::vector> argNames; }; + // One Op::CallModule site -- a ModularCall statically resolved (at + // compile time, via the enclosing ModuleDeclaration's own static + // scope) to a specific user ModuleDeclaration. Unlike CallSite, + // there's no isBuiltin/isImport branch to represent: a callee that + // ISN'T a resolved user ModuleDeclaration (a builtin, children(), an + // unresolvable name) never gets one of these -- it's compiled as a + // NativeStatement instead (see Op::CallModule's own doc comment for + // why that's exactly right, not a missed optimization). + struct ModuleCallSite { + const oscad::ModuleDeclaration* decl = nullptr; + const oscad::ModularCall* callNode = nullptr; + std::string calleeName; + }; + + // Every distinct chunk this SPECIFIC CompiledChunk owns is either a + // function body (bodyCode is one expression's own instruction stream) + // or a module body (bodyCode is a compiled STATEMENT list -- see + // tryCompileModuleBody, bytecode_compiler.cpp) -- never both. `params`/ + // `defaultCode` mean the same thing either way (a ModuleDeclaration's + // parameters are shaped identically to a FunctionDeclaration's). + // Gates driveVm's own frame-completion branch (bytecode_vm.cpp):a + // function frame's completion produces a Value (popped off f.stack); + // a module frame's produces nothing on the stack at all -- its whole + // effect already landed in treeStack_ as a side effect of running its + // own body. + bool isModule = false; + // The FunctionDeclaration/FunctionLiteral this chunk's own body was // compiled from (bytecode_compiler.cpp's compileFunctionLike sets this // to its own `selfDecl` parameter) -- Op::MakeClosure's runtime handler @@ -357,6 +508,12 @@ struct CompiledChunk { std::vector upvalues; std::vector closureSites; std::vector echoSites; + // Module-body-only tables (see Op::CallModule/NativeStatement/ + // NativeCondJumpIfFalse/NativeCheckDebugExprLevel/NativeIterMaterialize's + // own doc comments, above) -- always empty for a function chunk. + std::vector moduleCallSites; + std::vector nativeExprs; + std::vector nativeStatements; int numSlots = 0; // Count of distinct ListCompFor-assignment iterLists allocated across // this whole chunk (see IterMaterialize/IterReset/IterNext) -- runChunk diff --git a/include/openscad_cpp_evaluator/bytecode_compiler.hpp b/include/openscad_cpp_evaluator/bytecode_compiler.hpp index 55a3a4c..8d50cb8 100644 --- a/include/openscad_cpp_evaluator/bytecode_compiler.hpp +++ b/include/openscad_cpp_evaluator/bytecode_compiler.hpp @@ -3,20 +3,100 @@ #include "openscad_cpp_evaluator/bytecode.hpp" #include +#include namespace oscad { +class Assignment; class FunctionDeclaration; +class ModuleDeclaration; +class Expression; +class Scope; } // namespace oscad namespace oscadeval { // Attempts to compile `decl`'s parameter defaults + body to bytecode. -// Returns nullopt if `decl` uses any construct Phase 1 doesn't compile yet -// (a call of any kind, a function-literal value, a list comprehension, -// echo()/assert(), or a $-prefixed parameter) -- the caller falls back to -// the ordinary AST interpreter for that whole function, unconditionally -// correct since nothing about this function's behavior changes based on -// whether it happened to compile. +// Returns nullopt if `decl` uses a construct this compiler doesn't handle +// yet -- the caller falls back to the ordinary AST interpreter for that +// whole function, unconditionally correct since nothing about this +// function's behavior changes based on whether it happened to compile. std::optional tryCompileFunction(const oscad::FunctionDeclaration& decl); +// Compiles a bare STATEMENT-context expression -- an assignment's RHS, an +// if/for condition, a module-call or echo()/assert() argument -- rather +// than a whole function body. No parameters, no enclosing/upvalue chain, no +// self declaration: every name reference resolves through Op::LoadFree/ +// Op::LoadDyn, the identical ctx.let_/ctx.dyn lookup evalExpr's own +// Identifier case already does for a statement-level local (a for-loop +// variable, an assignment-declared name) -- this buys back the AST +// dispatch overhead compileExpr already saves for functions, not faster +// variable access (statement-level locals were never slot-addressable to +// begin with; they're declared by scattered Assignment statements, not a +// fixed parameter list known in advance). A nested let() inside the +// expression still gets a real compile-time slot, exactly as it would +// inside a function body. +// +// Refuses to compile (returns nullopt) if the expression creates any +// closure that captures something (chunk.closureSites non-empty after +// compileFunctionLike returns) -- with no enclosing chain and no self +// declaration for this bare wrapper, such a closure's own upvalue +// resolution would target a CallStackFrame that will never exist (this +// path pushes none), which is silently WRONG (stale/undef) rather than a +// safe fallback. Not a hypothetical carve-out: it's the one shape this +// wrapper cannot support correctly, so it's excluded rather than risked. +// `scope` is the expression's own lexical scope (Expression::scope(), +// always set by buildScope() regardless of node kind) -- needed to resolve +// any callee inside it statically, the same way a function body's call +// sites are. +std::optional tryCompileStatementExpr(const oscad::Expression& expr, const oscad::Scope* scope); + +// Compiles a run of SIBLING assignment statements sharing one scope -- +// exactly Evaluator::evalChildren's own `assignments` sub-list (stmt_eval. +// cpp), which OpenSCAD already runs as a self-contained group before any +// non-assignment statement in the same block. Unlike tryCompileStatementExpr +// (one independent chunk per leaf expression -- fine for a single +// assignment, but its own per-call overhead dominated a WHOLE block of +// them, see this feature's own commit message for the measured regression), +// this compiles the ENTIRE run as one chunk with one shared CompileScope: +// each assignment's own name gets a real slot (declared in source order, +// mirroring LetOp's own sequential-visibility rule -- `y = x + 1;` after +// `x = 1;` reads `x` via a fast LoadLocal, not a name lookup), and each +// value is written out via Op::StoreLocalAndLet so code OUTSIDE this batch +// still sees it the ordinary way (see runCompiledAssignmentBlock's own doc +// comment, bytecode_vm.hpp, for why `ctx` is used directly, not a scoped +// child). +// +// Refuses to compile (returns nullopt) for any of: +// - the same plain (non-$) name assigned more than once in this run -- +// evalAssignment's own "was assigned on line N but overwritten" warning +// would otherwise silently stop firing for a reassignment inside a +// compiled batch; safer to fall back than to lose it silently. +// - any closure that captures something (same hazard, same reasoning, as +// tryCompileStatementExpr's own doc comment above). +// - a $-prefixed assignment nested inside a let()/list-comprehension-let +// clause ANYWHERE in this run (post-compile: any Op::StoreDyn in the +// resulting bodyCode) -- runCompiledAssignmentBlock runs directly against +// the caller's own `ctx`, not a scoped child the way a bare expression +// chunk gets (letChildCtx(), see runCompiledExprChunk's own doc comment) +// -- there is no scope here for that nested let()'s own dyn write to be +// contained by, so it would leak into the caller's ctx.dyn permanently +// instead of just for that one assignment's own RHS. +std::optional tryCompileAssignmentBlock(const std::vector& assigns, + const oscad::Scope* scope); + +// Attempts to compile `decl`'s parameter defaults + STATEMENT-list body +// (decl.children) to bytecode (Stage 2) -- the module-side analog of +// tryCompileFunction. Returns nullopt (falls back to the ordinary +// interpreter for this whole module) for a construct compileStatementList +// doesn't handle -- see its own doc comment (bytecode_compiler.cpp) for +// exactly which statement kinds get real bytecode (assignment/if/for/a +// resolved user-module call) versus a native passthrough (everything +// else: echo/assert/let-blocks/modifiers/intersection_for/builtin or +// unresolved module calls) -- the native passthrough shapes never bail +// compilation of the WHOLE body the way an unsupported EXPRESSION +// construct does for tryCompileFunction; only a genuinely unsupported +// EXPRESSION (inside an if-condition/for-range/argument) can still bail +// (via tryCompileStatementExpr's own NotCompilable, propagated up). +std::optional tryCompileModuleBody(const oscad::ModuleDeclaration& decl); + } // namespace oscadeval diff --git a/include/openscad_cpp_evaluator/bytecode_vm.hpp b/include/openscad_cpp_evaluator/bytecode_vm.hpp index 5f3284a..15fd7c7 100644 --- a/include/openscad_cpp_evaluator/bytecode_vm.hpp +++ b/include/openscad_cpp_evaluator/bytecode_vm.hpp @@ -7,101 +7,238 @@ #include "openscad_cpp_parser/ast/expression.hpp" +#include #include +#include #include namespace oscadeval { class Evaluator; -// A reusable scratch buffer set for one compiled-call activation: the VM -// operand stack, the parameter/local slot array, and the "was this param -// actually bound by the caller" mask. Pooled on Evaluator (see -// acquireVmFrame()/releaseVmFrame()) rather than freshly heap-allocated per -// call -- profiling found 3 fresh vector allocations per call was enough to -// erase the whole point of avoiding bindArgs' single unordered_map -// allocation (see this project's own session notes / CLAUDE.md for the -// measured-not-assumed finding that motivated this). Pooling is safe under -// recursion: a still-executing (nested/recursive) call's own frame is -// never in the pool to be handed out again -- it's only returned once that -// call's own RAII guard destructs, and each acquire either reuses an -// already-returned frame or allocates a genuinely new one, never aliasing -// a frame two active calls both hold. +// One ListCompFor/statement-for assignment's own materialized iteration +// state -- see Op::IterMaterialize/IterReset/IterNext's own doc comments +// (bytecode.hpp). Lives in the header (not bytecode_vm.cpp's own anonymous +// namespace, where it used to be a purely internal type) because VmFrame, +// below, now owns one vector of these per frame -- iteration state belongs +// to whichever logical call is running it, exactly like `slots`/`stack` +// already do, so it has to travel with the frame across a push/pop the same +// way. +struct IterList { + IterableValues values; + size_t index = 0; + // Cached once at IterMaterialize time -- IterableValues::size() is O(1) + // for a list/owned vector, but walks a RANGE's whole sequence from + // scratch every call. Recomputing it once per IterNext call turned a + // single range-based for()/list-comp `for` into an O(n^2) walk (a + // 100,000-element range: interpreter ~0.03s, VM ~9.5s) -- caught via a + // BOSL2 corpus sweep (test_math.scadtest's own gaussian_rands(), which + // builds a 100,000-element list via `count()`). + size_t total = 0; +}; + +// One activation record in the explicit, heap-allocated VM call stack +// (Evaluator::vmCallStack_) that replaced native C++ recursion for calls +// between two compiled chunks -- see this project's own session notes for +// the full "iterate over the code, don't use the C++ call stack" rationale. +// Pooled (Evaluator::acquireVmFrame()/releaseVmFrame(), a vmFramePool_ of +// unique_ptr) rather than freshly heap-allocated per logical call +// -- profiling found 3 fresh vector allocations per call was enough to +// erase the whole point of the VM (measured, not assumed) -- `slots`/ +// `stack` are `.assign()`/`.clear()`-reset on reuse rather than +// reallocated, exactly as before this redesign. +// +// `chunk` vs. `code`: `chunk` is this frame's own CompiledChunk (constants/ +// names/callSites/closureSites/etc.); `code` is the SPECIFIC instruction +// vector actually being run -- almost always `&chunk->bodyCode`, but a +// parameter-default-value evaluation runs `&chunk->defaultCode[i]` instead +// (a genuinely different instruction sequence belonging to the SAME +// chunk). Kept as two separate pointers rather than assuming "code is +// always this chunk's own body" specifically so default-value evaluation +// can be driven by the exact same push/pop machinery as everything else +// (including a call made FROM a default value, however rare) instead of a +// second, parallel execution path. +// +// `ctxChain`: NEVER overwrite `ctxChain.back()` in place to "reset" a +// frame's context on a tail hop -- EvalContext's own let_/dyn/dynExplicit +// are shared_ptr> views onto a shared, mutable +// ScopeTrailStorage, and ~TrailView() pops its own trail level the INSTANT +// its last reference drops (scope_trail.hpp). Overwriting `ctx` in place +// would drop the old EvalContext's refcount to zero immediately, popping a +// trail level a still-reachable $-var lookup may need to walk back through +// later in the same tail chain -- exactly the bug +// runCompiledFunctionTrampoline's own (now-retired) `chain` vector existed +// to prevent, one layer down. Every hop (tail or not) does +// `ctxChain.push_back(newCtx)`; "current ctx" is always `ctxChain.back()`; +// torn down back-to-front (never relying on the vector's own unspecified +// destruction order -- libstdc++/libc++ disagree, see issue #50) whenever +// this frame is popped, on every exit path including exceptions. +// +// Whether (and under what name) THIS frame carries its own callStack_/ +// profiling bracket lives OUTSIDE VmFrame itself, in a parallel vector on +// Evaluator (vmCallBrackets_, pushed/popped in lockstep with +// vmCallStack_ by the driver) -- VmFrame can't hold an +// Evaluator::UserCallHandle directly (evaluator.hpp includes THIS header, +// so the reverse include would be circular). A frame gets a real bracket +// (via Evaluator::enterUserCall) only when it represents a genuine NEW +// logical function/closure call -- pushed by the driver's own non-tail +// Op::CallFn/CallDynamic handling, exactly mirroring what +// evalUserFunctionFromBound/evalFunctionLiteralFromBound used to do via a +// native recursive call. A "bare" frame (no entry in vmCallBrackets_) -- +// the very first frame of a top-level runCompiledFunction(FromBound) call +// (its own bracket belongs to the OUTER evalUserFunctionCore/ +// enterUserCall pair that invoked it), a bare statement-context expression +// (runCompiledExprChunk), an assignment block (runCompiledAssignmentBlock), +// or a parameter-default-value evaluation -- never touches callStack_/ +// profiling on its own, matching each of those call shapes' existing +// (pre-redesign) behavior exactly. struct VmFrame { - std::vector stack; + const CompiledChunk* chunk = nullptr; + const std::vector* code = nullptr; + size_t pc = 0; std::vector slots; + std::vector stack; std::vector bound; -}; + std::vector> accumStack; + std::vector iterLists; + std::vector ctxChain; + // The ORIGINAL callee name at push time, used by + // Evaluator::exitUserCallSuccess's own returnHook call when this frame + // carries a bracket -- deliberately NOT updated by a later tail hop + // within this SAME frame (recordTailCallHop only mutates callStack_'s + // own entry), mirroring evalUserFunctionCore's existing behavior of + // always reporting the original entry name regardless of how many tail + // hops happened inside it. Unused for a bare frame. + std::string logicalName; + // Evaluator::recordTailCallHop's own runaway-loop counter (the + // existing 1,000,000-iteration cap, user_calls.cpp) -- used to live as + // a local variable in the now-retired runCompiledFunctionTrampoline's + // own while loop, persisting for that trampoline's whole run; now + // persists here instead, since a tail hop replaces THIS frame in place + // rather than looping in a separate function. Reset to 0 whenever this + // frame is (re)constructed for a NEW logical call (a tail hop within + // an already-running frame does NOT reset it -- same "one counter per + // trampoline run" semantics as before). + unsigned tailHopGuard = 0; + + // -- Module-body compilation (Stage 2) -------------------------------- + // True only for a frame pushed by Op::CallModule (bytecode_vm.cpp) -- + // i.e. a NESTED module call made from within another already-compiled + // module body, where there's no native evalModularCall left around it + // to do the splice-vs-union post-processing. driveVm's own completion + // branch checks this to decide whether popping this frame should ALSO + // run Evaluator::spliceModuleChildren before resuming the caller. + // False for runCompiledModuleBody's own (bare) top-level frame -- its + // splice responsibility belongs to whatever NATIVE evalModularCall + // called Evaluator::evalUserModule in the first place, exactly + // mirroring runCompiledFunction's own bare/bracket-owned-by-caller + // split (see VmFrame's own doc comment above, and enterUserCall's + // skipDepthGuard). + bool ownsModuleSplice = false; + // Evaluator::randsCallCount_ captured at THIS frame's own push time + // (mirrors evalModularCall's own `randsBefore` local) -- only + // meaningful when ownsModuleSplice is true; read back by + // spliceModuleChildren when this frame completes. + std::uint64_t moduleRandsBefore = 0; + // The ModularCall AST node this frame's own splice (if any) should + // stamp onto a synthetic ">1 spliced child" union-wrapper CSGNode -- + // mirrors evalModularCall's own `&node` (csg_resolve.cpp). Only + // meaningful when ownsModuleSplice is true. + const oscad::ASTNode* moduleSpliceCallNode = nullptr; -// Filled by runChunk's Op::CallFnTail/CallDynamicTail handler when a tail -// hop is eligible to trampoline (isolated -- see Evaluator:: -// isolatedCallCtxFor's own doc comment) instead of recursing for real. -// `ctx` is the ALREADY-DERIVED isolated-call EvalContext (callCtxFor's own -// callCtx() branch, computed once by isolatedCallCtxFor rather than -// re-derived by the trampoline loop) -- a fresh $-var (dyn) trail level -// for this hop, matching what a real (non-tail) call would have built via -// evalUserFunctionFromBound/evalFunctionLiteralFromBound's own callCtxFor -// call. Exactly one of `decl`/`literal` is non-null; both null means "no -// tail request" (the sentinel runCompiledFunctionTrampoline/ -// runCompiledFunctionFromBoundTrampoline check for after each hop). -struct TailCallRequest { - const oscad::FunctionDeclaration* decl = nullptr; - const oscad::FunctionLiteral* literal = nullptr; - BoundArgs bound; - EvalContext ctx; - std::string name; - const oscad::Position* callPos = nullptr; + // Whether callStack_.back() (at push time) genuinely IS this frame's + // own logical call, and therefore safe for a tail hop inside this + // frame to overwrite via recordTailCallHop. This is NOT the same + // question as "does this frame own a vmCallBrackets_ entry to + // release" (Evaluator::vmCallBrackets_, see its own push sites) -- + // runCompiledFunction/runCompiledFunctionFromBound's own initial + // frame is bare (no entry of ITS own in vmCallBrackets_, since the + // bracket belongs to the CALLER's enterUserCall, already active + // before either function is even invoked) but callStack_.back() is + // still correctly THIS call the moment either one starts running -- + // so a tail call made directly from a top-level compiled function's + // own body must still be hop-eligible, exactly like a nested + // pushBracketedCallFrame call. Only genuinely call-boundary-free + // pushes (runCompiledExprChunk, runCompiledAssignmentBlock, a + // parameter default's own nested frame) leave this false -- for + // those, callStack_.back() (if anything) belongs to some unrelated + // enclosing call, and recordTailCallHop must not touch it. Found via + // BytecodeCompiler.ClosureWithDollarParameterNowCompilesToo: without + // this distinction EVERY top-level compiled call's own first tail + // call fell through to a genuine (bracketed) push instead of hopping, + // a correct-but-wasteful extra frame+checkDebug stop, not a value bug. + bool hopEligible = false; }; -// Binds `arguments` (evaluated against `callerCtx`, exactly like bindArgs) -// into a fresh slot array per chunk.params, applies any unbound parameter's -// compiled default, then runs the compiled body -- all against `childCtx` -// (the same callCtxFor()-derived context evalUserFunction already builds, -// for dyn/scope/color/childrenNodes; the interpreter's ctx.let_ is never -// written to for a compiled call, since every plain local this chunk knows -// about lives in the slot array instead). Mirrors evalUserFunction's own -// bindArgs+bound-loop+applyDefaults+evalExpr sequence exactly in observable -// effect, just via slots instead of a per-call unordered_map + ctx.let_. -// `tailOut`: forwarded to the body's own runChunk call -- see -// runCompiledFunctionTrampoline for the only caller that passes non-null. +// Binds `arguments` (raw AST, evaluated against `callerCtx`) into a fresh +// compiled call and runs it to completion -- replaces +// runCompiledFunctionTrampoline (retired, folded in here: tail hops are +// now handled entirely internally by the shared VM driver, never surfaced +// to callers). Mirrors evalUserFunction's own bindArgs+bound-loop+ +// applyDefaults+evalExpr sequence exactly in observable effect, just via +// slots instead of a per-call unordered_map + ctx.let_. This call's OWN +// callStack_/profiling bracket is the CALLER's (evalUserFunctionCore, +// already active by the time this runs) -- this function pushes a single +// BARE frame (see VmFrame::callBracket's own doc comment) and drives it; +// only a NON-TAIL call made from within its body to another compiled +// function gets its own bracket, pushed by the driver itself. Value runCompiledFunction(Evaluator& ev, const CompiledChunk& chunk, const std::vector>& arguments, EvalContext& callerCtx, - EvalContext& childCtx, TailCallRequest* tailOut = nullptr); + EvalContext& childCtx); // Same, but for a call site whose arguments are already evaluated (a -// compiled CALL_FN opcode's own callee, or Evaluator::evalUserFunctionFromBound's -// interpreter-callee sibling path) -- binds `bound`'s entries into slots by -// matching CompiledChunk::Param::name (an undeclared $-prefixed entry goes -// straight to childCtx.dyn, exactly like bindCompiledArgs' own handling; -// an undeclared plain name has no slot and is simply unreferenceable, same -// reasoning as bindCompiledArgs), applies defaults for anything left -// unbound, then runs the body. `tailOut`: see runCompiledFunction's own -// note -- this is also the function every trampoline hop AFTER the first -// reuses directly (see runCompiledFunctionTrampoline/ -// runCompiledFunctionFromBoundTrampoline), since a tail request's own -// bound-args shape is identical to this function's own parameter shape. +// compiled Op::CallFn opcode's own callee, or +// Evaluator::evalUserFunctionFromBound's interpreter-callee sibling path, +// or a tail hop) -- binds `bound`'s entries into slots by matching +// CompiledChunk::Param::name (an undeclared $-prefixed entry goes straight +// to childCtx.dyn; an undeclared plain name has no slot and is simply +// unreferenceable), applies defaults for anything left unbound, then runs +// the body to completion. Replaces runCompiledFunctionFromBoundTrampoline +// (retired) for the same reason as runCompiledFunction, above. Value runCompiledFunctionFromBound(Evaluator& ev, const CompiledChunk& chunk, const BoundArgs& bound, - EvalContext& childCtx, TailCallRequest* tailOut = nullptr); + EvalContext& childCtx); + +// Runs a bare statement-context chunk (see tryCompileStatementExpr, +// bytecode_compiler.hpp) -- no parameters to bind, no defaults, never +// itself a tail-hop target. `ctx` is used AS-IS, not derived via +// callCtxFor -- a statement-context expression runs in the same scope as +// the statement it belongs to, not a new call boundary, exactly like the +// interpreter's own evalExpr(*node.expr, ctx) call it replaces. A call +// made FROM within this expression to a compiled function still gets the +// full explicit-frame-stack treatment (pushed by the shared driver) -- +// only THIS bare wrapper frame itself carries no callStack_/profiling +// bracket of its own. +Value runCompiledExprChunk(Evaluator& ev, const CompiledChunk& chunk, EvalContext& ctx); + +// Runs a compiled ASSIGNMENT BLOCK (see Evaluator::tryCompileAssignmentBlock, +// user_calls.cpp) -- a run of sibling assignment statements sharing one +// scope. Unlike runCompiledExprChunk, `ctx` is used directly, NOT via a +// fresh letChildCtx(): every Op::StoreLocalAndLet writes straight into +// ctx.let_ (and any $-assignment, if this block has one, straight into +// ctx.dyn via Op::StoreDyn) precisely so those writes OUTLIVE this call and +// stay visible to whatever runs after it in the same scope -- the entire +// point of compiling the block at all. Safe from the same nested-`let()` +// dyn-leak hazard runCompiledExprChunk's own letChildCtx() guards against +// only because tryCompileAssignmentBlock refuses to compile a block +// containing one in the first place (scans the compiled bodyCode for any +// Op::StoreDyn). +void runCompiledAssignmentBlock(Evaluator& ev, const CompiledChunk& chunk, EvalContext& ctx); -// The trampoline entry points -- replace runCompiledFunction/ -// runCompiledFunctionFromBound at evalUserFunction's/evalUserFunctionFromBound's/ -// evalFunctionLiteral's/evalFunctionLiteralFromBound's own call sites -// (user_calls.cpp). Runs the first (real) call normally, then loops on -// any TailCallRequest that comes back -- each subsequent hop reuses -// runCompiledFunctionFromBound directly (a tail request's bound-args -// shape already matches its own parameter shape exactly), acquiring a -// fresh VmFrame each time (releasing the previous one -- safe, since only -// an ISOLATED hop ever reaches here, meaning nothing can hold an upvalue -// into a tail-discarded frame's own slots) and recording the hop via -// Evaluator::recordTailCallHop (callStack_ frame mutation, the -// 1,000,000-iteration cap, profiling). See these functions' own .cpp -// definitions for why every hop's own EvalContext must be kept alive for -// the whole trampoline run (a std::vector chain, mirroring -// evalFunctionBodyTrampoline's identical fix on the interpreter side). -Value runCompiledFunctionTrampoline(Evaluator& ev, const CompiledChunk& chunk, - const std::vector>& arguments, - EvalContext& callerCtx, EvalContext& childCtx); -Value runCompiledFunctionFromBoundTrampoline(Evaluator& ev, const CompiledChunk& chunk, const BoundArgs& bound, - EvalContext& childCtx); +// Runs a compiled MODULE body (see tryCompileModuleBody, bytecode_ +// compiler.hpp) to completion -- the module-side analog of +// runCompiledFunction/runCompiledFunctionFromBound. Pushes a single BARE +// frame (ownsModuleSplice=false: this call's own splice responsibility +// belongs to whichever NATIVE evalModularCall invoked +// Evaluator::evalUserModule, which already pushed treeStack_'s own +// accumulator before calling in here) and drives it; only a NESTED +// Op::CallModule made from within its body gets its own bracket+splice +// responsibility, pushed by the driver itself. `childCtx` is the fully +// prepared child scope (parameters bound, $children/$parent_modules set, +// defaults applied) -- see Evaluator::buildModuleChildCtx, which both +// this entry point's caller (evalUserModule) and Op::CallModule's own +// handler use to build it identically either way. No return value: a +// module body's whole effect is the side effect of whatever CSGNodes +// landed in treeStack_ while it ran. +void runCompiledModuleBody(Evaluator& ev, const CompiledChunk& chunk, EvalContext& childCtx); } // namespace oscadeval diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 669aa7e..1777719 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -100,6 +100,10 @@ class Evaluator { // tagGenerated()/builtinChildren() are: the call site is a free // function, not a method. void noteRandsCall() { ++randsCallCount_; } + // Read-only accessor for the same reason: Op::CallModule's own runtime + // handler (bytecode_vm.cpp) needs its own "randsBefore" snapshot, + // mirroring evalModularCall's own local of the same name. + std::uint64_t randsCallCount() const { return randsCallCount_; } // Dispatches on node.kind() (switch, not a visitor -- matches // openscad_cpp_parser's own dispatch convention). Throws @@ -110,6 +114,24 @@ class Evaluator { // answer. Value evalExpr(const oscad::Expression& node, EvalContext& ctx); + // Statement-context sibling of evalExpr -- same result for every input, + // just a possibly-compiled path for a STATEMENT-level expression + // (assignment RHS, if/for condition, module-call or echo()/assert() + // argument) instead of module/top-level code's ordinary AST walk. Not a + // new evaluation semantics: looks up (compiling and caching on first + // use, see stmtExprChunkCache_/tryCompileStatementExpr) a bytecode + // chunk for `node`, runs it via runCompiledExprChunk when the VM is on, + // this specific chunk is debugger-eligible right now (chunkEligibleNow, + // the identical per-chunk gate every compiled function call already + // goes through), AND a resolveTree()/evaluate() pass is actually + // active (inResolvePass_ -- see stmtExprChunkCache_'s own doc comment + // for why caching outside one is unsafe) -- falling straight back to + // evalExpr(node, ctx) otherwise. Compilation failing, the VM being off, + // a debugger needing this exact span, or no active pass are all + // silently equivalent to "just interpret it," same as functions' own + // fallback. + Value evalExprMaybeCompiled(const oscad::Expression& node, EvalContext& ctx); + // Evaluates a block's statements in OpenSCAD's assignment-before- // geometry order (all Assignment nodes first, then everything else, // each group preserving source order), each against its own lexical @@ -478,8 +500,26 @@ class Evaluator { Value evalAssertExpr(const oscad::AssertOp& node, EvalContext& ctx); void bindLetName(EvalContext& ctx, const std::string& name, const Value& v); + // Bracketed public in place: Stage 2's Op::NativeStatement + // (bytecode_vm.cpp, a separate translation unit) needs it directly to + // run one native-passthrough statement from inside an otherwise- + // compiled module body -- exactly the same dispatch evalChildren's own + // per-statement loop already does. +public: void evalStatement(const oscad::ASTNode& node, EvalContext& ctx); +private: void evalAssignment(const oscad::Assignment& node, EvalContext& ctx); + // Tries to compile+run `assignments` (evalChildren's own leading + // assignment-run, stmt_eval.cpp) as ONE chunk via + // tryCompileAssignmentBlock/runCompiledAssignmentBlock -- see their own + // doc comments (bytecode_compiler.hpp/bytecode_vm.hpp) and + // assignBlockChunkCache_'s, above. Returns false (nothing run, `ctx` + // untouched) whenever compiling/running the WHOLE batch this way isn't + // possible or eligible right now, so the caller can fall back to its + // existing per-statement loop -- exactly evalExprMaybeCompiled's own + // "silently equivalent to interpreting it" contract, just for a whole + // block instead of one leaf expression. + bool tryRunCompiledAssignmentBlock(const std::vector& assignments, EvalContext& ctx); void evalModularCall(const oscad::ModularCall& node, EvalContext& ctx); void evalFor(const oscad::ModularFor& node, EvalContext& ctx); void evalLetBlock(const oscad::ModularLet& node, EvalContext& ctx); @@ -509,6 +549,7 @@ class Evaluator { // -- User function/module calls (Phase 4) -------------------------- +public: // Picks between an isolated call scope (callCtx()) and a closure- // capturing one (childCtx()) for entering `decl`'s body: walks the // live call stack, and if `decl`'s own declaration span is *strictly* @@ -551,6 +592,7 @@ class Evaluator { const EvalContext* childrenCallerCtx = nullptr, bool* usedChildCtx = nullptr, const std::shared_ptr>& capturedLet = nullptr); +private: // Fills in any parameter not already present in `bound` (bindArgs' // own return value -- the authoritative "did the caller actually // supply this name" record) from its own default-value expression, @@ -629,68 +671,30 @@ class Evaluator { // closure type all the way through, so the compiler can call (and // often inline) `computeResult()` directly -- no erasure, no // allocation, same one-call-site-per-caller shape as before. + // A synchronous call-and-block wrapper around enterUserCall/ + // exitUserCallSuccess/exitUserCallException (public, below) -- kept as + // the ordinary entry point for the tree-walking interpreter's own + // recursive call sites (user_calls.cpp), which genuinely do want "call + // and block for the result" semantics. The explicit-frame-stack VM + // driver (bytecode_vm.cpp) calls the two halves directly instead, + // since ITS whole point is a compiled-to-compiled call must NOT block + // a native frame waiting for the callee -- see enterUserCall's own doc + // comment for the split rationale. This wrapper's own behavior is + // byte-for-byte identical to the pre-split version; the split changed + // nothing observable here. template Value evalUserFunctionCore(const std::string& name, const oscad::ASTNode& declNode, const oscad::Expression& bodyExpr, EvalContext& childCtx, const oscad::Position* callPos, int upvalueParent, F&& computeResult) { - // Guards against a genuinely non-tail-recursive user function -- - // each logical call HERE is a real, unavoidable C++ recursive call - // (a trampolined tail call never re-enters this function at all, - // see evalFunctionBodyTrampoline/runCompiledFunctionTrampoline) -- - // silently overflowing the native stack: a hard, unrecoverable - // process crash with no C++ exception to catch. Confirmed present - // (before this guard existed) for a plain, closure-free `function - // f(n) = n<=0 ? 0 : 1+f(n-1);`, compiled OR interpreted. - // - // kMaxUserCallDepth is a conservative, heuristic cap, NOT a - // precisely-calibrated one -- and specifically calibrated against - // the WORST measured platform, not the best. Two rounds of CI - // diagnostics (temporary test builds that recursed to increasing - // depths, printing success after each one so the CI log itself - // would show exactly where a crash happened) found: (1) this - // evaluator's own C++ recursion is safe into the THOUSANDS on an - // ordinary macOS/Linux desktop main-thread stack, but only into - // the low HUNDREDS on CI's windows-latest runner (smaller default - // thread stack and/or larger MSVC-compiled call frames than - // Clang's); (2) actually THROWING from that depth and unwinding - // back out through that many nested try/catch frames (this guard - // fires from inside evalUserFunctionCore, which every level of - // the recursion has its own try/catch in) costs meaningfully MORE - // native stack than simply being that deep without throwing -- - // 300 survived on Windows without throwing, but throwing+ - // unwinding through as few as 100 nested frames did not (80 was - // the last depth that survived). 50 leaves real margin below that - // 80 figure for whatever's already on the stack from this call's - // own caller, and for a smaller stack still (e.g. BelfrySCAD's own - // debug-session worker thread, plausibly smaller than any CI - // runner's main thread) that this evaluator has no way to probe - // for directly. A real OpenSCAD/BOSL2 recursive function is - // overwhelmingly tail-recursive in practice anyway (exactly why - // reduce()/accumulate()/while() and this evaluator's own - // trampolines exist) -- genuine non-tail recursion even - // approaching this depth is already the rare, likely-runaway case - // a controlled error serves far better than a crash. - static constexpr size_t kMaxUserCallDepth = 50; - if (callStack_.size() >= kMaxUserCallDepth) { - error("Recursion too deep while calling function '" + name + "'", declNode); - } - std::optional prof = profileEnter("function", name, callPos, &declNode.position()); - callStack_.push_back(CallStackFrame{CallStackFrame::Kind::Function, name, callPos, &declNode.position(), &declNode, - nullptr, upvalueParent}); - callStack_.back().bodyCtx = &childCtx; // per-frame locals for the debugger + UserCallHandle h = enterUserCall(name, declNode, &bodyExpr, childCtx, callPos, upvalueParent); Value result; try { - checkDebug(bodyExpr, childCtx); - lastCtx_ = &childCtx; result = computeResult(); - if (debugHooks_.returnHook) debugHooks_.returnHook(name, result, static_cast(callStack_.size())); } catch (...) { - callStack_.pop_back(); - if (prof) profileExit(*prof); + exitUserCallException(h); throw; } - callStack_.pop_back(); - if (prof) profileExit(*prof); + exitUserCallSuccess(name, h, result); return result; } @@ -775,6 +779,53 @@ class Evaluator { Value evalFunctionBodyTrampoline(const oscad::Expression& bodyExpr, EvalContext& ctx); void evalUserModule(const oscad::ModuleDeclaration& decl, const oscad::ModularCall& call, EvalContext& ctx); + // Bracketed public in place: Op::CallModule's own runtime handler and + // driveVm's module-frame completion branch (bytecode_vm.cpp, a + // separate translation unit) need buildModuleChildCtx/ + // runModuleBodyNative/lookupOrCompileModuleChunk/spliceModuleChildren + // directly -- the same reason callCtxFor/enterUserCall/useBytecodeVm + // were bracketed public for Stage 1's own driver. +public: + // The shared setup evalUserModule's own compiled-or-native dispatch AND + // Op::CallModule's runtime handler (bytecode_vm.cpp) both need, + // factored out so neither duplicates it: resolves the module's own + // child scope, builds childrenNodes, derives childCtx via callCtxFor, + // sets $children/$parent_modules, binds `bound`'s entries and applies + // defaults. Exactly what evalUserModule used to do inline before Stage + // 2 split it out -- see this file's own git history for the pre-split + // version if a byte-for-byte diff is ever needed. + EvalContext buildModuleChildCtx(const oscad::ModuleDeclaration& decl, const oscad::ModularCall& call, + EvalContext& ctx, BoundArgs bound); + // Runs `decl`'s own body natively against the already-prepared + // `childCtx` -- depth guard, callStack_/profiling bracket (Kind:: + // Module), evalChildren, teardown. This is exactly what evalUserModule + // used to do unconditionally before Stage 2 taught it to try a + // compiled chunk first (see lookupOrCompileModuleChunk) -- still the + // fallback whenever `decl` doesn't compile, called both from + // evalUserModule itself and from Op::CallModule's own "callee doesn't + // compile" branch. + void runModuleBodyNative(const oscad::ModuleDeclaration& decl, EvalContext& childCtx, const oscad::Position* callPos); + // Compile-attempt cache for module bodies, mirroring chunkCache_ + // exactly (nullopt = tried, doesn't compile). See tryCompileModuleBody + // (bytecode_compiler.hpp). + const CompiledChunk* lookupOrCompileModuleChunk(const oscad::ModuleDeclaration& decl); + // Taints every one of `children` if `randsBefore` differs from the + // current randsCallCount_, then either wraps >1 sibling into a + // synthetic "union" CSGNode (stamped with `callNode`) or splices each + // child directly into treeStack_.back() -- the CALLER's own frame, + // already exposed at the top of treeStack_ by the time this runs + // (`children` itself came from popping the CALLEE's own frame, already + // done by the caller before this runs -- see each call site). Factored + // out of evalModularCall's own post-processing (csg_resolve.cpp) so + // Op::CallModule's own frame-completion (bytecode_vm.cpp's driveVm) + // can reuse it exactly -- always the splice branch in practice for a + // caller that reached here via a resolved user-module call + // (evalModularCall's OWN "is this splice or wrap-as-a-tagged-node" + // branch already decided `splice` before ever reaching a user module, + // so this helper never needs that decision itself). + void spliceModuleChildren(std::vector> children, std::uint64_t randsBefore, + const oscad::ASTNode& callNode); +private: Value evalUserFunction(const std::string& name, const oscad::FunctionDeclaration& decl, const std::vector>& arguments, EvalContext& ctx, const oscad::ASTNode* callNode); @@ -793,6 +844,11 @@ class Evaluator { // uses e.g. echo() isn't re-attempted on every single call. std::unordered_map> chunkCache_; + // Same, for module bodies (Stage 2) -- see tryCompileModuleBody + // (bytecode_compiler.hpp) and lookupOrCompileModuleChunk's own doc + // comment, above. + std::unordered_map> moduleChunkCache_; + // FunctionLiteral chunks (Phase 2/closures): populated only as a side // effect of successfully compiling some OTHER declaration that // lexically contains the literal (see CompiledChunk::nestedLiterals' @@ -807,6 +863,63 @@ class Evaluator { std::unordered_map literalChunkCache_; void flattenNestedLiterals(CompiledChunk& chunk); + // Statement-context expression chunks (Phase 1, module/top-level + // compilation) -- see tryCompileStatementExpr's own doc comment + // (bytecode_compiler.hpp) and evalExprMaybeCompiled's, below. Keyed by + // expression node identity, same nullopt-means-tried-and-failed + // convention as chunkCache_ above (an expression using a construct this + // wrapper doesn't support isn't re-attempted on every statement it's + // part of). + // + // UNLIKE chunkCache_/literalChunkCache_ (keyed by a FunctionDeclaration/ + // FunctionLiteral -- a NAMED, script-lifetime declaration), a bare + // statement-context Expression node has no such guarantee: it's only + // alive as long as the AST it's part of. A caller that re-parses and + // re-evaluates DIFFERENT, independently-lived expressions against one + // reused Evaluator (several existing tests do exactly this, and it's + // not implausible for real usage -- e.g. a debugger REPL evaluating an + // ad-hoc watch expression) can free one Expression node and have a + // LATER parse's allocator hand back the exact same address for an + // unrelated node -- stmtExprChunkCache_.find() would then silently + // return a chunk compiled for the WRONG expression. Caught for real: + // MathBuiltins.AbsSignCeilFloorRound-style tests returning values from + // an entirely different, already-freed call's own constant. + // + // Kept safe by TWO measures, both required: resolveTreeImpl (csg_ + // resolve.cpp) clears this at the start of every top-level resolveTree()/ + // evaluate() call (bounds staleness to "within one render pass" even + // when a host reuses one Evaluator across separate, re-parsed renders), + // and evalExprMaybeCompiled only reads/writes it at all while + // inResolvePass_ is true (bounds it away entirely for any direct + // evalExpr()-family call made outside a resolveTree()/evaluate() pass -- + // exactly the ad-hoc/test-helper shape above, which has no pass + // boundary to tie cache validity to in the first place). + std::unordered_map> stmtExprChunkCache_; + + // Assignment-BLOCK chunks (see tryCompileAssignmentBlock's own doc + // comment, bytecode_compiler.hpp, and tryRunCompiledAssignmentBlock's, + // below) -- a whole run of sibling assignments compiled as ONE chunk, + // not stmtExprChunkCache_'s one-chunk-per-leaf-expression (whose own + // per-call overhead turned out to dominate for a typical block of + // several small assignments -- see this feature's own commit message + // for the measured regression that motivated batching them instead). + // Keyed by the run's own FIRST assignment -- stable and unique per + // block: evalChildren's own assignments/others split is deterministic + // over a fixed AST, so a given block's leading-assignment-run always + // has the same first member across repeated evaluations. Same nullopt- + // means-tried-and-failed convention, same dangling-pointer hazard and + // same two-part fix (cleared alongside stmtExprChunkCache_ at the top + // of every resolveTreeImpl call, only read/written while + // inResolvePass_) as stmtExprChunkCache_ itself -- see its own doc + // comment above for the full reasoning. + std::unordered_map> assignBlockChunkCache_; + + // See stmtExprChunkCache_'s own doc comment, immediately above, for why + // this exists. Not a reentrancy guard (resolveTreeImpl is never called + // while another one is already active), just an on/off switch for + // whether evalExprMaybeCompiled's cache is safe to touch right now. + bool inResolvePass_ = false; + public: // lookupOrCompileChunk/lookupCompiledLiteralChunk: public (not just // private helpers of evalUserFunction*/evalFunctionLiteral* anymore) @@ -943,10 +1056,19 @@ class Evaluator { // debugger happens to be attached somewhere" wastes the most time // (see docs/debugger.md and this method's own callers for the full // story). +public: + // Public (not just a private helper of evalUserFunction*/ + // evalFunctionLiteral* anymore): the explicit-frame-stack driver + // (bytecode_vm.cpp, a separate translation unit) needs this same gate + // for a NESTED call made from within already-compiled code, exactly + // the same decision evalUserFunction*/evalFunctionLiteral* already + // make before calling lookupOrCompileChunk/lookupCompiledLiteralChunk + // themselves. bool useBytecodeVm() const { return bytecodeVmEnabled() && (!debugHooks_.debugHook || fastContinueBreakpoints_.has_value()); } +private: // The fine-grained half of the check above: even when useBytecodeVm() // says compiling/using bytecode is on the table at all, a SPECIFIC // chunk is only actually safe to run compiled if nothing about it @@ -1009,6 +1131,141 @@ class Evaluator { void profileRecordTailHop(const std::string& kind, const std::string& name, const oscad::Position* callPos, const oscad::Position* declPos); +public: + // -- User-call bracket, split into enter/exit halves ------------------ + // (explicit-frame-stack VM groundwork) + // + // evalUserFunctionCore (above) used to inline this whole bracket + // (depth-guard check, profileEnter, callStack_ push, checkDebug, + // computeResult() called SYNCHRONOUSLY in the same native frame, + // returnHook, callStack_ pop, profileExit) as one block wrapping a + // caller-supplied lambda. That shape is fine for the tree-walking + // interpreter, which always wants genuine call-and-block semantics -- + // but it can't be reused by an explicit-frame-stack bytecode VM driver + // (bytecode_vm.cpp), whose entire point is that a compiled-to-compiled + // call must NOT block a native C++ frame waiting for the callee's + // result; the driver instead needs to PUSH a new frame and return + // control to its own outer loop, applying "what happens on return" + // later, when that pushed frame's own execution reaches its end. + // + // Split into two explicit halves so both callers -- evalUserFunctionCore + // itself (a thin synchronous wrapper around them, see above, kept for + // the interpreter's own recursive call sites) and the VM driver's own + // push/pop points -- can each drive the SAME bracket logic without + // duplicating it. `enterUserCall` does everything up to and including + // the body-entry checkDebug() stop; `exitUserCallSuccess`/ + // `exitUserCallException` do the matching teardown, mirroring + // evalUserFunctionCore's own try/catch split exactly (returnHook fires + // ONLY on the success path, matching today's behavior precisely). + struct UserCallHandle { + std::optional prof; + // Mirrors the pushed CallStackFrame's own kind -- exitUserCall* + // needs it to decrement moduleCallDepth_ correctly without + // re-reading callStack_ (already popped by the time exitUserCall* + // runs). See moduleCallDepth_'s own doc comment for why this + // exists at all. + CallStackFrame::Kind kind = CallStackFrame::Kind::Function; + // The declaration this call pushed, for the SAME reason as + // `kind` above: exitUserCall* needs it to decrement + // activeDeclRefcount_'s own count for exactly this declaration, + // and callStack_.back() is already gone by the time it runs. See + // activeDeclRefcount_'s own doc comment. + const oscad::ASTNode* declNode = nullptr; + }; + // `skipDepthGuard`: kMaxUserCallDepth=50 exists purely to keep the + // NATIVE C++ stack from overflowing (see its own doc comment) -- true + // only for pushBracketedCallFrame's/pushBracketedModuleFrame's own + // calls (bytecode_vm.cpp), a compiled-to-compiled push that never + // grows the native stack at all (it's serviced by driveVm's own loop, + // bounded instead by kMaxVmCallStackDepth, checked separately by the + // caller before this runs). evalUserFunctionCore's own call (the + // interpreter-fallback boundary, still a genuine native recursive + // call either way) always passes false, unchanged. `kind`: Function + // for every existing call site; Module for pushBracketedModuleFrame's + // own (Stage 2) -- selects both the CallStackFrame::Kind pushed (so + // $parent_modules/backtraces count a compiled module call exactly + // like a native evalUserModule one) and the profiling site's own + // "function"/"module" label. `bodyExpr`: nullptr for a module call -- + // unlike a function, the native evalUserModule path fires no + // "body entry" checkDebug of its own at all (each of the module's own + // STATEMENT children gets one instead, via Op::NativeStatement's own + // runtime handler / evalChildren's per-statement loop) -- passing a + // real bodyExpr here for a module call would add a stop that never + // existed before. + UserCallHandle enterUserCall(const std::string& name, const oscad::ASTNode& declNode, + const oscad::Expression* bodyExpr, EvalContext& childCtx, + const oscad::Position* callPos, int upvalueParent, bool skipDepthGuard = false, + CallStackFrame::Kind kind = CallStackFrame::Kind::Function); + // `fireReturnHook`: false only for a module frame's own completion -- + // the native evalUserModule path never calls debugHooks_.returnHook + // either (a module call has no "return value" concept the debugger + // reports), so a compiled module call must not start doing so just + // because it happens to reuse this same exit path. + void exitUserCallSuccess(const std::string& name, const UserCallHandle& handle, const Value& result, + bool fireReturnHook = true); + void exitUserCallException(const UserCallHandle& handle); + + // Decrements activeDeclRefcount_[declNode], erasing the entry once it + // reaches 0 (keeps the map's own size bounded by the number of + // CURRENTLY active distinct declarations, not the number ever seen) -- + // shared by exitUserCallSuccess/exitUserCallException/ + // recordTailCallHop's own "retire the old declaration" half. + void noteActiveDeclExit(const oscad::ASTNode* declNode); + + // The CALLER's own callStack_ index right before a new frame is pushed + // for it -- exactly the `callerFrameIdx` every evalUserFunction* + // variant already computes inline (user_calls.cpp) before deciding + // upvalueParent, needed here too by the explicit-frame-stack driver's + // own push helper (a free function in a separate translation unit, + // bytecode_vm.cpp, so it can't read callStack_ directly). Must be + // read BEFORE enterUserCall's own callStack_.push_back(), same + // ordering constraint the inline computation already has. + int callStackTopIndex() const { return callStack_.empty() ? -1 : static_cast(callStack_.size()) - 1; } + + // The explicit, heap-allocated VM call stack that replaced native C++ + // recursion for a call between two compiled chunks -- see this + // project's own session notes ("iterate over the code, don't use the + // C++ call stack for recursion") and VmFrame's own doc comment + // (bytecode_vm.hpp) for the full rationale. Public, and manipulated + // directly (push_back/pop_back/back()) by bytecode_vm.cpp's driver + // loop rather than through narrow wrapper methods -- that driver IS + // this stack's only real owner/user, in a hot loop, the same pragmatic + // choice already made for treeStack_ (csg_resolve.cpp). A + // unique_ptr has an address independent of the surrounding + // vector -- reallocating `vmCallStack_` itself never moves a VmFrame + // in memory, so CallStackFrame::vmFrame (a raw pointer into whichever + // element is current) stays valid exactly as it always has. + std::vector> vmCallStack_; + + // Parallel to vmCallStack_ (same size, same push/pop points, always) -- + // holds this frame's own callStack_/profiling bracket handle when it + // has one (nullopt for a "bare" frame: a top-level compiled-call entry + // point's own first frame, a bare statement expression, an assignment + // block, a parameter default) -- see VmFrame's own doc comment for why + // this can't just be a VmFrame member (evaluator.hpp can't be + // forward-referenced from bytecode_vm.hpp, which evaluator.hpp itself + // includes). + std::vector> vmCallBrackets_; + + // Separate from kMaxUserCallDepth (which stays exactly as-is, still + // guarding the interpreter-fallback/native-recursion boundary -- native + // stack margin is still the real concern there). This one gates a + // PUSH onto vmCallStack_ itself: unbounded growth from a genuinely + // runaway non-tail-recursive compiled function is now a heap/memory + // concern, not a native-stack-overflow one, so it needs a much higher + // ceiling -- reusing kTcoIterationCap's own figure (the tail-loop + // runaway guard, user_calls.cpp) for consistency rather than inventing + // an unrelated third number. Revisit with the same CI-diagnostic-loop + // method already used for kMaxUserCallDepth if this ever needs + // recalibrating for real. + static constexpr size_t kMaxVmCallStackDepth = 1'000'000; + + // Bracketed public in place: Op::CallModule's own runtime handler and + // driveVm's module-frame completion branch (bytecode_vm.cpp, a + // separate translation unit) push/pop this directly -- exactly the + // same "that driver IS this stack's own only other real owner/user" + // reasoning vmCallStack_ was already made public for (Stage 1). +public: // The tree-build stack (mirrors the reference's self._tree_stack): each // frame is the children accumulator for whichever CSGNode is currently // being resolved (or, at the bottom, the top-level tree itself). A @@ -1020,6 +1277,8 @@ class Evaluator { // push/call/pop sequence. std::vector>> treeStack_; +private: + // User function/module call stack: (kind, name, call position, decl // position) per active call, innermost last. Drives callCtxFor's // closure detection and error()'s TRACE lines. Mirrors the reference's @@ -1027,6 +1286,60 @@ class Evaluator { // eval_error.hpp). std::vector callStack_; + // Running count of Module-kind frames currently on callStack_ -- + // maintained incrementally by enterUserCall/exitUserCall* rather than + // rescanned from callStack_ on every module call (buildModuleChildCtx's + // own $parent_modules) the way it used to be. That rescan is O(depth) + // per call, so a script recursing deep enough to actually need Stage + // 2's own new depth ceiling turned it into real O(depth^2) wall time -- + // measured directly: a script recursing through recur() alone (no + // geometry cost at all) went from 0.49s at depth 16,000 to 7.19s at + // depth 64,000 (quadratic, not the expected ~4x-for-4x-the-work + // linear scaling) before this fix. Harmless at the OLD native- + // recursion depths (capped at kMaxUserCallDepth=50, this scan was + // never more than 50 entries) -- only became a real problem once + // Stage 2 made much deeper module recursion possible at all. + int moduleCallDepth_ = 0; + + // Refcounted set of DISTINCT declarations currently active anywhere on + // callStack_ -- keyed by declaration ASTNode identity (the same + // pointer CallStackFrame::declNode/enterUserCall's own `declNode` use + // throughout), maintained incrementally by enterUserCall/ + // exitUserCall*/recordTailCallHop (a tail hop swaps the ACTIVE + // declaration for the SAME stack slot without a matching enter/exit + // pair of its own, so it has to update this too, symmetrically: + // decrement the frame's old declNode, increment the new one). + // + // callCtxFor's own closure/lexical-nesting detection used to walk the + // FULL callStack_ testing every single frame's declPosition for span + // containment -- O(depth) per call, invisible at the old native- + // recursion-bound depths (~50) but genuinely quadratic once compiled + // calls (Stage 1/2, see this project's own session notes) made much + // deeper recursion possible: a plain non-tail-recursive `f(n) = n<=0 + // ? 0 : 1+f(n-1)` went 10k->0.15s, 20k->0.54s, 40k->2.1s (measured -- + // textbook quadratic, not the expected ~2x-per-2x-depth linear + // scaling). The fix exploits a structural fact about REAL recursion: + // a deep call chain is overwhelmingly the SAME declaration repeated + // many times, not many DISTINCT declarations -- so the number of + // DISTINCT active declarations stays small and roughly constant + // regardless of recursion depth, even though callStack_ itself grows + // unboundedly. Since containment for a given declaration X depends + // only on X's own (compile-time-fixed) position, never on WHICH of + // its (possibly many) active occurrences is checked, checking each + // DISTINCT X once is exactly equivalent to checking every occurrence + // -- not an approximation. + std::unordered_map activeDeclRefcount_; + + // Shared cap on callStack_'s own size, checked by both + // evalUserFunctionCore (below) and evalUserModule (user_calls.cpp) -- + // see evalUserFunctionCore's own doc comment for the full calibration + // story (two rounds of CI stack-depth diagnostics on the worst-case + // platform). One constant because both push onto the SAME callStack_ -- + // a function recursing into a module recursing into a function is the + // same native-stack-growth hazard regardless of which kind of frame is + // on top at any given depth. + static constexpr size_t kMaxUserCallDepth = 50; + // See lastChildrenPositions()'s own doc comment. std::optional>> lastChildrenPositions_; diff --git a/pyproject.toml b/pyproject.toml index 51952e1..7eadbfe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.10.0" +version = "0.11.0" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/builtins/call_args.cpp b/src/builtins/call_args.cpp index 10f2032..a47c278 100644 --- a/src/builtins/call_args.cpp +++ b/src/builtins/call_args.cpp @@ -47,10 +47,10 @@ CallArgs resolveArgs(Evaluator& ev, const std::vectorkind() == oscad::NodeKind::PositionalArgument) { auto& a = static_cast(*argPtr); - result.positional.emplace_back(pos++, ev.evalExpr(*a.expr, ctx)); + result.positional.emplace_back(pos++, ev.evalExprMaybeCompiled(*a.expr, ctx)); } else if (argPtr->kind() == oscad::NodeKind::NamedArgument) { auto& a = static_cast(*argPtr); - result.named.emplace_back(a.name->name, ev.evalExpr(*a.expr, ctx)); + result.named.emplace_back(a.name->name, ev.evalExprMaybeCompiled(*a.expr, ctx)); } } return result; diff --git a/src/bytecode_compiler.cpp b/src/bytecode_compiler.cpp index 9c9788e..b8285fe 100644 --- a/src/bytecode_compiler.cpp +++ b/src/bytecode_compiler.cpp @@ -5,6 +5,7 @@ #include "openscad_cpp_parser/ast/declarations.hpp" #include "openscad_cpp_parser/ast/expression.hpp" +#include "openscad_cpp_parser/ast/module_instantiation.hpp" #include "openscad_cpp_parser/ast/vector_element.hpp" #include "openscad_cpp_parser/scope.hpp" @@ -229,6 +230,30 @@ class Compiler { return slot; } + // Shared by the Identifier case (below) and PrimaryCall's own callee + // probe (a bare-identifier callee that didn't resolve to a builtin/user + // function statically -- "maybe it's a closure value") -- same name + // resolution either way, only the terminal free-variable fallback + // differs: an ordinary read always warns on a genuinely unknown name + // (LoadFree), but the callee probe must not (LoadFreeNoWarn) -- + // mirrors evalFunctionCall's own `evalIdentifier(..., warnIfUndef)` + // split exactly (user_calls.cpp). See LoadFreeNoWarn's own doc comment, + // bytecode.hpp, for the double-warning bug this fixes. + void compileIdentifierLoad(const std::string& name, const oscad::Position& pos, std::vector& out, + CompileScope& scope, bool warnIfUndef) { + if (!name.empty() && name[0] == '$') { + out.push_back({Op::LoadDyn, internName(name), 0, &pos}); + } else if (auto slot = scope.resolve(name)) { + out.push_back({Op::LoadLocal, *slot, 0, &pos}); + } else if (auto upvalSlot = resolveEnclosing(name)) { + int idx = static_cast(chunk_.upvalues.size()); + chunk_.upvalues.push_back(*upvalSlot); + out.push_back({Op::LoadUpvalue, idx, 0, &pos}); + } else { + out.push_back({warnIfUndef ? Op::LoadFree : Op::LoadFreeNoWarn, internName(name), 0, &pos}); + } + } + // Widens chunk_'s own [minLine, maxLine] span to include `node` -- // called at the top of every AST-node-visiting entry point // (compileExpr/compileListElement/compileListCompBody) so the chunk's @@ -308,17 +333,7 @@ class Compiler { return; case NodeKind::Identifier: { auto& n = static_cast(node); - if (!n.name.empty() && n.name[0] == '$') { - out.push_back({Op::LoadDyn, internName(n.name), 0, &n.position()}); - } else if (auto slot = scope.resolve(n.name)) { - out.push_back({Op::LoadLocal, *slot, 0, &n.position()}); - } else if (auto upvalSlot = resolveEnclosing(n.name)) { - int idx = static_cast(chunk_.upvalues.size()); - chunk_.upvalues.push_back(*upvalSlot); - out.push_back({Op::LoadUpvalue, idx, 0, &n.position()}); - } else { - out.push_back({Op::LoadFree, internName(n.name), 0, &n.position()}); - } + compileIdentifierLoad(n.name, n.position(), out, scope, /*warnIfUndef=*/true); return; } case NodeKind::ListComprehension: { @@ -560,7 +575,26 @@ class Compiler { // otherwise) -- see Op::CallDynamic's own runtime // handler. Never itself in tail position (it's the // CALLEE being resolved, not this call's own result). - compileExpr(*n.left, out, scope); + // + // A bare-identifier callee (leftId) goes through + // compileIdentifierLoad directly with warnIfUndef=false + // (LoadFreeNoWarn for the free-variable fallback, + // matching evalFunctionCall's own + // evalIdentifier(..., /*warnIfUndef=*/false) probe) -- + // NOT the generic compileExpr(*n.left, ...), which + // would use plain Identifier compilation (LoadFree, + // warnIfUndef=true) and double up "unknown variable" + // on top of CallDynamic's own "unknown function" + // warning for a genuinely unresolvable name. Anything + // else (index/member access, another call's result) + // has no such special-cased probe in the interpreter + // either -- ordinary compileExpr, whatever warnings + // THAT naturally produces. + if (leftId) { + compileIdentifierLoad(leftId->name, leftId->position(), out, scope, /*warnIfUndef=*/false); + } else { + compileExpr(*n.left, out, scope); + } } for (const auto& argPtr : n.arguments) { @@ -1062,6 +1096,156 @@ class Compiler { compileListElement(body, out, scope); } + // -- Module-body statement-list compilation (Stage 2) ----------------- + // See tryCompileModuleBody's own doc comment (bytecode_compiler.hpp) + // for the overall shape: assignment/if/for get real bytecode (Jump- + // based control flow, so a recursive module call nested inside one + // doesn't hide behind a native call boundary); a resolved user-module + // call gets Op::CallModule; everything else -- echo/assert/let-blocks/ + // modifiers/intersection_for, a builtin or unresolved module call, a + // plain assignment's own STORE (its RHS value, unlike a function's, + // is never slot-addressed here -- see this file's own module-chunk + // doc comment, bytecode.hpp, for why module bodies never allocate + // slots at all) -- is a native passthrough (Op::NativeStatement), + // exactly the same dispatch evalChildren's own per-statement loop + // already does for that one node. + + int internNativeExpr(const oscad::Expression* expr) { + chunk_.nativeExprs.push_back(expr); + return static_cast(chunk_.nativeExprs.size()) - 1; + } + int internNativeStatement(const oscad::ASTNode* node) { + chunk_.nativeStatements.push_back(node); + return static_cast(chunk_.nativeStatements.size()) - 1; + } + + // Mirrors evalChildren's own assignments-then-others partition + // (stmt_eval.cpp) exactly -- OpenSCAD runs every assignment in a scope + // before anything else, each group preserving its own source order. + // Reordering at COMPILE time (rather than delegating each statement + // in SOURCE order and relying on some runtime reordering) is what lets + // every statement -- assignment or not -- collapse to the same simple + // "compile once, in the right position" treatment. + void compileStatementList(const std::vector>& children, std::vector& out) { + std::vector assignments; + std::vector others; + for (const auto& c : children) { + (c->kind() == oscad::NodeKind::Assignment ? assignments : others).push_back(c.get()); + } + for (const oscad::ASTNode* stmt : assignments) compileOneStatement(*stmt, out); + for (const oscad::ASTNode* stmt : others) compileOneStatement(*stmt, out); + } + + void compileOneStatement(const oscad::ASTNode& stmt, std::vector& out) { + using oscad::NodeKind; + trackSpan(stmt); + switch (stmt.kind()) { + // Pure declarations, no-ops at statement-eval time (already + // hoisted into scope by buildScopes()) -- matches evalStatement's + // own default case; not even worth a NativeStatement entry. + case NodeKind::ModuleDeclaration: + case NodeKind::FunctionDeclaration: + return; + case NodeKind::ModularCall: { + auto& call = static_cast(stmt); + const oscad::Scope* lookupScope = stmt.scope() ? stmt.scope() : scope_; + const oscad::ASTNode* resolved = lookupScope ? lookupScope->lookupModule(call.name->name) : nullptr; + if (resolved && resolved->kind() == NodeKind::ModuleDeclaration) { + CompiledChunk::ModuleCallSite site; + site.decl = static_cast(resolved); + site.callNode = &call; + site.calleeName = call.name->name; + chunk_.moduleCallSites.push_back(std::move(site)); + out.push_back( + {Op::CallModule, static_cast(chunk_.moduleCallSites.size()) - 1, 0, &call.position()}); + return; + } + // Builtin, children(), or a name that didn't resolve to a + // user module statically -- native passthrough, exactly + // like echo/assert/etc. below. Never the recursion-depth + // risk this compiler targets (see NativeStatement's own + // doc comment, bytecode.hpp). + out.push_back({Op::NativeStatement, internNativeStatement(&stmt), 0, &stmt.position()}); + return; + } + case NodeKind::ModularIf: { + auto& n = static_cast(stmt); + const int jumpFalseIdx = static_cast(out.size()); + out.push_back({Op::NativeCondJumpIfFalse, internNativeExpr(n.condition.get()), 0, &n.condition->position()}); + const oscad::ASTNode* marker = n.trueBranch.empty() ? &stmt : n.trueBranch.front().get(); + out.push_back({Op::NativeCheckDebugExprLevel, internNativeStatement(marker), 0, nullptr}); + compileStatementList(n.trueBranch, out); + out[static_cast(jumpFalseIdx)].b = static_cast(out.size()); + return; + } + case NodeKind::ModularIfElse: { + auto& n = static_cast(stmt); + const int jumpFalseIdx = static_cast(out.size()); + out.push_back({Op::NativeCondJumpIfFalse, internNativeExpr(n.condition.get()), 0, &n.condition->position()}); + const oscad::ASTNode* trueMarker = n.trueBranch.empty() ? &stmt : n.trueBranch.front().get(); + out.push_back({Op::NativeCheckDebugExprLevel, internNativeStatement(trueMarker), 0, nullptr}); + compileStatementList(n.trueBranch, out); + const int jumpEndIdx = static_cast(out.size()); + out.push_back({Op::Jump, 0, 0, nullptr}); + out[static_cast(jumpFalseIdx)].b = static_cast(out.size()); + const oscad::ASTNode* falseMarker = n.falseBranch.empty() ? &stmt : n.falseBranch.front().get(); + out.push_back({Op::NativeCheckDebugExprLevel, internNativeStatement(falseMarker), 0, nullptr}); + compileStatementList(n.falseBranch, out); + out[static_cast(jumpEndIdx)].a = static_cast(out.size()); + return; + } + case NodeKind::ModularFor: { + compileForLoop(static_cast(stmt), out); + return; + } + default: + // echo/assert/let-block/modifiers/intersection_for -- + // deliberately native, see this section's own header + // comment above. + out.push_back({Op::NativeStatement, internNativeStatement(&stmt), 0, &stmt.position()}); + return; + } + } + + // Cartesian nested loop over N assignments, unrolled into flat Jump- + // based code at COMPILE time (bounded by the source's own for-clause + // count, always small -- never runtime-driven) rather than the + // interpreter's own runtime recursion (evalFor's `recurse(depth+1, + // ...)`, stmt_eval.cpp). Each dimension: materialize once (native RHS + // eval + expandIterable), then a ForIterNext/ForIterEnd pair wrapping + // everything inner -- see Op::ForIterNext/ForIterEnd's own doc + // comments (bytecode.hpp) for exactly why a fresh per-iteration ctx + // (not an in-place mutation) is required for correctness, not just + // parity. + void compileForLoop(const oscad::ModularFor& n, std::vector& out) { + const size_t numDims = n.assignments.size(); + std::vector iterListIds(numDims); + for (size_t d = 0; d < numDims; ++d) { + iterListIds[d] = nextIterList_++; + out.push_back({Op::NativeIterMaterialize, internNativeExpr(n.assignments[d]->expr.get()), + iterListIds[d], &n.assignments[d]->position()}); + } + std::vector topIdx(numDims); + std::vector forIterNextIdx(numDims); + for (size_t d = 0; d < numDims; ++d) { + topIdx[d] = out.size(); + forIterNextIdx[d] = out.size(); + Instruction ins; + ins.op = Op::ForIterNext; + ins.a = internName(n.assignments[d]->name->name); + ins.b = iterListIds[d]; + ins.node = n.assignments[d].get(); + out.push_back(ins); // ins.c (exhaustion target) patched below + } + const oscad::ASTNode* marker = n.body.empty() ? static_cast(&n) : n.body.front().get(); + out.push_back({Op::NativeCheckDebugExprLevel, internNativeStatement(marker), 0, nullptr}); + compileStatementList(n.body, out); + for (size_t i = numDims; i-- > 0;) { + out.push_back({Op::ForIterEnd, static_cast(topIdx[i]), 0, nullptr}); + out[forIterNextIdx[i]].c = static_cast(out.size()); + } + } + private: CompiledChunk& chunk_; const oscad::Scope* scope_; @@ -1118,4 +1302,93 @@ std::optional tryCompileFunction(const oscad::FunctionDeclaration return chunk; } +std::optional tryCompileStatementExpr(const oscad::Expression& expr, const oscad::Scope* scope) { + static const std::vector> kNoParams; + CompiledChunk chunk; + if (!compileFunctionLike(chunk, scope, nullptr, {}, kNoParams, expr)) return std::nullopt; + // See this function's own doc comment (bytecode_compiler.hpp) for why a + // captures-having nested closure can't be supported by this bare + // wrapper -- selfDecl is nullptr and enclosing is empty above, so its + // own upvalue(s) would target a CallStackFrame this path never pushes. + // A zero-capture closure never reaches closureSites at all (see the + // FunctionLiteral case's own PushConst early-return, above) so it's + // unaffected by this check. + if (!chunk.closureSites.empty()) return std::nullopt; + return chunk; +} + +std::optional tryCompileAssignmentBlock(const std::vector& assigns, + const oscad::Scope* scope) { + // Reassignment-warning fidelity (see this function's own doc comment, + // bytecode_compiler.hpp) -- cheap, one-time scan before touching the + // compiler at all. + std::unordered_set seenNames; + int topLevelDollarAssignments = 0; + for (const oscad::Assignment* a : assigns) { + const std::string& name = a->name->name; + if (!name.empty() && name[0] == '$') { + ++topLevelDollarAssignments; // evalAssignment never warns for a $-name either -- not tracked in seenNames + } else if (!seenNames.insert(name).second) { + return std::nullopt; + } + } + + CompiledChunk chunk; + Compiler compiler(chunk, scope, nullptr, {}); + CompileScope compileScope; + compileScope.push(); + try { + for (const oscad::Assignment* a : assigns) { + compiler.compileExpr(*a->expr, chunk.bodyCode, compileScope); // RHS is never tail + const std::string& name = a->name->name; + if (!name.empty() && name[0] == '$') { + chunk.bodyCode.push_back({Op::StoreDyn, compiler.internName(name), 0, &a->position()}); + } else { + int slot = compiler.declareLocal(compileScope, name); + chunk.bodyCode.push_back({Op::StoreLocalAndLet, slot, compiler.internName(name), &a->position()}); + } + } + } catch (const NotCompilable&) { + return std::nullopt; + } + chunk.numSlots = compiler.nextSlot(); + chunk.numIterLists = compiler.nextIterList(); + + // See this function's own doc comment for why: runCompiledAssignmentBlock + // (bytecode_vm.cpp) runs directly against the caller's own ctx, with no + // scope of its own for a nested let()'s dyn write to be contained by. + if (!chunk.closureSites.empty()) return std::nullopt; + // Every StoreDyn this loop itself emitted above is accounted for by + // topLevelDollarAssignments; any MORE than that in the compiled body + // can only have come from a nested let()/list-comprehension-let clause + // (StoreDyn's only other two emission sites) inside one of these + // assignments' own RHS -- refuse the whole block rather than risk that + // write leaking into the caller's ctx.dyn permanently. + int storeDynCount = 0; + for (const Instruction& ins : chunk.bodyCode) { + if (ins.op == Op::StoreDyn) ++storeDynCount; + } + if (storeDynCount != topLevelDollarAssignments) return std::nullopt; + return chunk; +} + +std::optional tryCompileModuleBody(const oscad::ModuleDeclaration& decl) { + CompiledChunk chunk; + chunk.isModule = true; + chunk.selfDecl = &decl; + // No params/defaultCode/numSlots here -- a module's own parameters are + // bound natively (Evaluator::buildModuleChildCtx, called by the + // CALLER before this chunk's own body ever runs), never slot- + // addressed the way a function's are. See this file's own module- + // chunk doc comment (bytecode.hpp) for the full reasoning. + Compiler compiler(chunk, decl.scope(), nullptr, {}); + try { + compiler.compileStatementList(decl.children, chunk.bodyCode); + } catch (const NotCompilable&) { + return std::nullopt; + } + chunk.numIterLists = compiler.nextIterList(); + return chunk; +} + } // namespace oscadeval diff --git a/src/bytecode_vm.cpp b/src/bytecode_vm.cpp index 5d43671..a0fa7be 100644 --- a/src/bytecode_vm.cpp +++ b/src/bytecode_vm.cpp @@ -7,52 +7,12 @@ #include "openscad_cpp_evaluator/scope_trail.hpp" #include "openscad_cpp_parser/ast/declarations.hpp" +#include "openscad_cpp_parser/ast/module_instantiation.hpp" namespace oscadeval { namespace { -// RAII pool checkout -- see VmFrame's own doc comment (bytecode_vm.hpp) for -// why pooling exists. Always returns its frame to the pool on destruction, -// including via exception unwind (a nested compiled call throwing through -// CallFn's own evalUserFunctionFromBound/evalBuiltinFunction call). -class VmFrameGuard { -public: - explicit VmFrameGuard(Evaluator& ev) : ev_(ev), frame_(ev.acquireVmFrame()) {} - ~VmFrameGuard() { ev_.releaseVmFrame(std::move(frame_)); } - VmFrameGuard(const VmFrameGuard&) = delete; - VmFrameGuard& operator=(const VmFrameGuard&) = delete; - - VmFrame& get() { return *frame_; } - -private: - Evaluator& ev_; - std::unique_ptr frame_; -}; - -// `stack` is a caller-owned, pooled scratch buffer (see VmFrameGuard) -- -// cleared here at entry rather than declared fresh, since one compiled -// call's own VmFrame is reused across each of its unbound-default -// evaluations AND its body (sequential, never concurrent, so sharing one -// buffer across those sub-runs is safe and avoids allocating a new stack -// vector for each). -// One ListCompFor assignment's own materialized iteration state -- see -// IterMaterialize/IterReset/IterNext's own doc comments (bytecode.hpp). -struct IterList { - IterableValues values; - size_t index = 0; - // Cached once at IterMaterialize time -- IterableValues::size() is O(1) - // for a list/owned vector, but walks a RANGE's whole sequence from - // scratch every call (its own doc comment: "no caller in this codebase - // actually calls size() on a range today" -- Op::IterNext, below, used - // to be exactly that stale caller). Recomputing it once per IterNext - // call turned a single range-based for()/list-comp `for` into an O(n^2) - // walk (a 100,000-element range: interpreter ~0.03s, VM ~9.5s) -- caught - // via a BOSL2 corpus sweep (test_math.scadtest's own gaussian_rands(), - // which builds a 100,000-element list via `count()`). - size_t total = 0; -}; - // Shared by Op::CallFn's isBuiltin (evalBuiltinFunction) and isImport // (importAsValue) branches -- both take the same pre-resolved CallArgs // shape resolveArgs would build for the interpreter path. `args` is @@ -62,9 +22,6 @@ CallArgs buildCallArgs(const CompiledChunk::CallSite& site, std::vector& int positionalIdx = 0; for (size_t i = 0; i < argCount; ++i) { if (site.argNames[i]) { - // Each call site's argNames are distinct source arguments, never - // a repeated key, so a direct emplace_back (no overwrite check) - // is safe and avoids a pointless linear scan on every insert. callArgs.named.emplace_back(*site.argNames[i], std::move(args[i])); } else { callArgs.positional.emplace_back(positionalIdx++, std::move(args[i])); @@ -73,552 +30,75 @@ CallArgs buildCallArgs(const CompiledChunk::CallSite& site, std::vector& return callArgs; } -Value runChunk(Evaluator& ev, const CompiledChunk& chunk, const std::vector& code, - std::vector& slots, EvalContext& ctx, std::vector& stack, - TailCallRequest* tailOut = nullptr) { - stack.clear(); - // Native scratch state for list-comprehension clauses (Phase 3) -- not - // pooled (unlike stack/slots/bound): only chunks containing a real - // comprehension clause ever touch these, and both start genuinely - // empty per runChunk call regardless, so there's no steady-state - // capacity to preserve across calls the way stack/slots benefit from. - std::vector> accumStack; - std::vector iterLists(static_cast(chunk.numIterLists)); - size_t pc = 0; - while (pc < code.size()) { - const Instruction& ins = code[pc]; - switch (ins.op) { - case Op::PushConst: - stack.push_back(chunk.constants[static_cast(ins.a)]); - ++pc; - break; - case Op::PushBool: - stack.push_back(Value{ins.a != 0}); - ++pc; - break; - case Op::LoadLocal: - stack.push_back(slots[static_cast(ins.a)]); - ++pc; - break; - case Op::StoreLocal: { - Value v = std::move(stack.back()); - stack.pop_back(); - slots[static_cast(ins.a)] = std::move(v); - ++pc; - break; - } - case Op::LoadDyn: { - const Value* v = ctx.dyn->find(chunk.names[static_cast(ins.a)]); - stack.push_back(v ? *v : Value{}); - ++pc; - break; - } - case Op::StoreDyn: { - Value v = std::move(stack.back()); - stack.pop_back(); - const std::string& name = chunk.names[static_cast(ins.a)]; - ctx.dyn->set(name, v); - ctx.dynExplicit->set(name, true); - ++pc; - break; - } - case Op::LoadFree: - stack.push_back(ev.evalIdentifier(chunk.names[static_cast(ins.a)], ins.pos, ctx, true)); - ++pc; - break; - case Op::LoadUpvalue: { - // findUpvalue walks the LIVE call stack -- correct as long - // as the declaring call is still active, however many - // upvalue levels out `uv.targetDecl` sits (an ordinary, - // not-yet-escaped nested closure, e.g. reduce()'s own - // self-recursive accumulator). If that frame is gone - // (this chunk is running as an ESCAPED closure's own body - // -- see the FunctionLiteral case's own doc comment, - // bytecode_compiler.cpp), fall back to `ctx.let_`: for any - // closure invocation with a non-null capturedLet, - // callCtxFor roots `ctx.let_` at that exact snapshot, which - // is guaranteed (by construction -- effectiveCaptures is a - // superset of this chunk's own upvalues) to contain this - // name. - const CompiledChunk::UpvalueRef& uv = chunk.upvalues[static_cast(ins.a)]; - const Value* v = ev.findUpvalue(uv.targetDecl, uv.slot); - if (!v) v = ctx.let_->find(uv.name); - stack.push_back(v ? *v : Value{}); - ++pc; - break; - } - case Op::MakeClosure: { - // Builds a FRESH captured environment every time this - // instruction actually runs (never a compile-time constant - // -- a loop creating one closure per iteration must capture - // THAT iteration's own values, not share one snapshot). - // Each capture is genuinely local to the CURRENTLY-RUNNING - // chunk only when its own `targetDecl` matches this chunk's - // `selfDecl` (this literal's direct free-variable - // references) -- read straight out of `slots` in that case. - // Anything else is a capture bubbled up from a literal - // nested even deeper (see ClosureSite's own doc comment, - // bytecode.hpp): resolved the same way Op::LoadUpvalue - // resolves any upvalue reference, above -- the live call - // stack first (still live if THIS closure hasn't itself - // escaped yet), `ctx.let_` otherwise (this chunk's own - // capturedLet, when it's running as an escaped closure's - // body). The resulting Closure::capturedLet is a real, - // standalone TrailView (isolated root -- nothing else - // shares it, and it owes nothing to `ctx.let_` itself, - // which a compiled call never writes to directly -- see - // bindCompiledArgs' own doc comment), so invoking this - // closure later works through the EXACT same - // callCtxFor/capturedLetTrail machinery every interpreter- - // created escaping closure already uses, unchanged. - const CompiledChunk::ClosureSite& site = chunk.closureSites[static_cast(ins.a)]; - auto capturedTrail = TrailView::makeRoot(); - // A letrec-style self-reference (UpvalueRef::isSelfReference, - // see its own doc comment) is skipped here, not read -- the - // closure it names is THIS instruction's own result, which - // doesn't exist until the shared_ptr below is constructed. - // Patched in immediately afterward via the exact same - // capturedTrail (a live, shared TrailView -- mutating it - // after `closure` wraps it is exactly as visible to that - // closure's own later invocations as any other entry). - std::vector selfNames; - for (const auto& cap : site.captures) { - if (cap.isSelfReference) { - selfNames.push_back(&cap.name); - continue; - } - if (cap.targetDecl == chunk.selfDecl) { - capturedTrail->set(cap.name, slots[static_cast(cap.slot)]); - continue; - } - const Value* v = ev.findUpvalue(cap.targetDecl, cap.slot); - if (!v) v = ctx.let_->find(cap.name); - capturedTrail->set(cap.name, v ? *v : Value{}); - } - auto closure = std::make_shared(Closure{site.node, capturedTrail}); - for (const std::string* selfName : selfNames) { - capturedTrail->set(*selfName, Value{closure}); - } - stack.push_back(Value{std::move(closure)}); - ++pc; - break; - } - case Op::PatchClosureCapture: { - // Mutual-recursion support (see the LetOp compile case's - // own doc comment, bytecode_compiler.cpp): `slots[ins.a]` - // (the consumer, e.g. isEven) already holds a real - // MakeClosure-created closure whose own capturedTrail is - // missing this ONE entry -- it was left out entirely at - // compile time because the sibling it names (e.g. isOdd) - // hadn't been constructed yet. `slots[ins.c]` (that - // sibling's own slot) does now, since this instruction only - // ever runs immediately after that sibling's own - // Op::StoreLocal. Mutates the SAME shared TrailView the - // consumer's own capturedLet already points at -- visible - // to it exactly like every other capture, no different - // from Op::MakeClosure's own self-reference patch, above. - const Value& consumerVal = slots[static_cast(ins.a)]; - if (const auto* closurePtr = std::get_if(&consumerVal); closurePtr && *closurePtr) { - if (auto trail = capturedLetTrail(**closurePtr)) { - trail->set(chunk.names[static_cast(ins.b)], slots[static_cast(ins.c)]); - } - } - ++pc; - break; - } - case Op::Range: { - Value step = std::move(stack.back()); - stack.pop_back(); - Value end = std::move(stack.back()); - stack.pop_back(); - Value start = std::move(stack.back()); - stack.pop_back(); - stack.push_back(ev.applyRange(start, end, step)); - ++pc; - break; - } - case Op::Index: { - Value idx = std::move(stack.back()); - stack.pop_back(); - Value obj = std::move(stack.back()); - stack.pop_back(); - stack.push_back(ev.applyIndexAccess(obj, idx)); - ++pc; - break; - } - case Op::Member: { - Value obj = std::move(stack.back()); - stack.pop_back(); - stack.push_back(ev.applyMemberAccess(obj, chunk.names[static_cast(ins.a)])); - ++pc; - break; - } - case Op::UnaryOp: { - Value v = std::move(stack.back()); - stack.pop_back(); - stack.push_back(ev.applyUnaryOp(static_cast(ins.a), v, *ins.pos)); - ++pc; - break; - } - case Op::BinaryOp: { - Value b = std::move(stack.back()); - stack.pop_back(); - Value a = std::move(stack.back()); - stack.pop_back(); - stack.push_back(ev.applyBinaryOp(static_cast(ins.a), a, b, *ins.pos)); - ++pc; - break; - } - case Op::Jump: - pc = static_cast(ins.a); - break; - case Op::JumpIfFalse: { - Value v = std::move(stack.back()); - stack.pop_back(); - if (!truthy(v)) { - pc = static_cast(ins.a); - } else { - ++pc; - } - break; - } - case Op::JumpIfTrue: { - Value v = std::move(stack.back()); - stack.pop_back(); - if (truthy(v)) { - pc = static_cast(ins.a); - } else { - ++pc; - } - break; - } - case Op::OpenLocalScope: { - for (int i = 0; i < ins.b; ++i) slots[static_cast(ins.a + i)] = Value{}; - ++pc; - break; - } - case Op::BuildList: { - const size_t count = static_cast(ins.a); - std::vector items(count); - for (size_t i = 0; i < count; ++i) { - items[count - 1 - i] = std::move(stack.back()); - stack.pop_back(); - } - stack.push_back(Value{std::make_shared(ValueList{std::move(items)})}); - ++pc; - break; - } - case Op::AccumOpen: - accumStack.emplace_back(); - ++pc; - break; - case Op::AccumAppendOne: { - Value v = std::move(stack.back()); - stack.pop_back(); - accumStack.back().push_back(std::move(v)); - ++pc; - break; - } - case Op::AccumAppendEach: { - Value v = std::move(stack.back()); - stack.pop_back(); - appendEachInto(accumStack.back(), v); - ++pc; - break; - } - case Op::AccumClose: { - std::vector items = std::move(accumStack.back()); - accumStack.pop_back(); - stack.push_back(Value{std::make_shared(ValueList{std::move(items)})}); - ++pc; - break; - } - case Op::AccumMergeEach: { - std::vector inner = std::move(accumStack.back()); - accumStack.pop_back(); - for (const Value& item : inner) appendEachInto(accumStack.back(), item); - ++pc; - break; - } - case Op::IterMaterialize: { - Value v = std::move(stack.back()); - stack.pop_back(); - IterList& il = iterLists[static_cast(ins.a)]; - il.values = expandIterable(v, [&](size_t count) { - ev.warn("Bad range parameter in for statement: too many elements (" + std::to_string(count) + ")", ins.pos); - }); - il.index = 0; - il.total = il.values.size(); // once here, never per-iteration -- see IterList's own doc comment - ++pc; - break; - } - case Op::IterReset: { - iterLists[static_cast(ins.a)].index = 0; - ++pc; - break; - } - case Op::IterNext: { - IterList& il = iterLists[static_cast(ins.b)]; - if (il.index < il.total) { - slots[static_cast(ins.a)] = il.values[il.index]; - ++il.index; - ++pc; - } else { - pc = static_cast(ins.c); - } - break; - } - case Op::CheckIterLimit: { - double count = std::get(slots[static_cast(ins.a)]) + 1.0; - slots[static_cast(ins.a)] = Value{count}; - if (count > static_cast(ins.b)) { - ev.error("C-style for loop exceeded maximum iteration count", *ins.node); - } - ++pc; - break; - } - case Op::CallFn: { - const CompiledChunk::CallSite& site = chunk.callSites[static_cast(ins.a)]; - const size_t argCount = static_cast(ins.b); - std::vector args(argCount); - for (size_t i = 0; i < argCount; ++i) { - args[argCount - 1 - i] = std::move(stack.back()); - stack.pop_back(); - } - Value result; - if (site.isImport) { - // import(): not in isBuiltinFunctionName's table (special- - // cased earlier in evalFunctionCall), but importAsValue - // takes the exact same pre-resolved CallArgs shape - // evalBuiltinFunction does. - CallArgs callArgs = buildCallArgs(site, args, argCount); - result = importAsValue(ev, callArgs, *site.callNode); - } else if (site.isBuiltin && site.calleeName == "object") { - // object() needs its arguments merged in exact call-site - // interleaved order, which CallArgs' split positional/ - // named maps can't represent -- but args/site.argNames - // are already in that exact source order (compiled and - // popped the same way every other call's arguments are), - // so no raw AST access is needed here at all. - std::vector, Value>> pairs; - pairs.reserve(argCount); - for (size_t i = 0; i < argCount; ++i) pairs.emplace_back(site.argNames[i], std::move(args[i])); - result = mergeObjectArgs(pairs); - } else if (site.isBuiltin) { - // evalBuiltinFunction takes a pre-resolved CallArgs, no - // live ctx needed at all -- builtin functions are - // already value-based, no interpreter bridge required. - CallArgs callArgs = buildCallArgs(site, args, argCount); - result = evalBuiltinFunction(ev, site.calleeName, callArgs, *site.callNode); - } else { - // Replays bindArgs' own positional/named matching rule - // (positional index counted only among positional - // arguments, later write for a repeated name wins) - // against already-evaluated values. - BoundArgs bound; - bound.reserve(argCount); - size_t positionalIdx = 0; - const size_t nparams = site.decl->parameters.size(); - for (size_t i = 0; i < argCount; ++i) { - if (site.argNames[i]) { - bound.set(*site.argNames[i], std::move(args[i])); - } else { - if (positionalIdx < nparams) { - bound.set(site.decl->parameters[positionalIdx]->name->name, std::move(args[i])); - } - ++positionalIdx; - } - } - result = ev.evalUserFunctionFromBound(site.calleeName, *site.decl, std::move(bound), ctx, - &site.callNode->position()); - } - stack.push_back(std::move(result)); - ++pc; - break; - } - case Op::CallDynamic: { - const CompiledChunk::CallSite& site = chunk.callSites[static_cast(ins.a)]; - const size_t argCount = static_cast(ins.b); - std::vector args(argCount); - for (size_t i = 0; i < argCount; ++i) { - args[argCount - 1 - i] = std::move(stack.back()); - stack.pop_back(); - } - Value callee = std::move(stack.back()); - stack.pop_back(); - Value result; - if (const auto* closurePtr = std::get_if(&callee); closurePtr && *closurePtr) { - const Closure& closure = **closurePtr; - BoundArgs bound; - bound.reserve(argCount); - size_t positionalIdx = 0; - const size_t nparams = closure.node->parameters.size(); - for (size_t i = 0; i < argCount; ++i) { - if (site.argNames[i]) { - bound.set(*site.argNames[i], std::move(args[i])); - } else { - if (positionalIdx < nparams) { - bound.set(closure.node->parameters[positionalIdx]->name->name, std::move(args[i])); - } - ++positionalIdx; - } - } - result = ev.evalFunctionLiteralFromBound(closure, std::move(bound), ctx, ins.pos); - } else if (!site.calleeName.empty()) { - ev.warn("Ignoring unknown function '" + site.calleeName + "'", ins.pos); - } - stack.push_back(std::move(result)); - ++pc; - break; - } - case Op::CallFnTail: { - const CompiledChunk::CallSite& site = chunk.callSites[static_cast(ins.a)]; - const size_t argCount = static_cast(ins.b); - std::vector args(argCount); - for (size_t i = 0; i < argCount; ++i) { - args[argCount - 1 - i] = std::move(stack.back()); - stack.pop_back(); - } - BoundArgs bound; - bound.reserve(argCount); - size_t positionalIdx = 0; - const size_t nparams = site.decl->parameters.size(); - for (size_t i = 0; i < argCount; ++i) { - if (site.argNames[i]) { - bound.set(*site.argNames[i], std::move(args[i])); - } else { - if (positionalIdx < nparams) { - bound.set(site.decl->parameters[positionalIdx]->name->name, std::move(args[i])); - } - ++positionalIdx; - } - } - // Eligible for trampolining only when isolated (not - // closure-nested -- see Evaluator::isolatedCallCtxFor's own - // doc comment) AND the callee itself has a compiled chunk - // (a tail hop into a callee that doesn't compile pays one - // real C++ frame at that boundary instead, same as today). - // Ineligible falls through to exactly what CallFn already - // does -- no new behavior for that case. - if (tailOut != nullptr) { - if (auto hopCtx = ev.isolatedCallCtxFor(*site.decl, ctx)) { - if (ev.lookupOrCompileChunk(*site.decl) != nullptr) { - tailOut->decl = site.decl; - tailOut->literal = nullptr; - tailOut->bound = std::move(bound); - tailOut->ctx = std::move(*hopCtx); - tailOut->name = site.calleeName; - tailOut->callPos = &site.callNode->position(); - return Value{}; - } - } - } - Value result = ev.evalUserFunctionFromBound(site.calleeName, *site.decl, std::move(bound), ctx, - &site.callNode->position()); - stack.push_back(std::move(result)); - ++pc; - break; +// Replays bindArgs' own positional/named matching rule (positional index +// counted only among positional arguments, later write for a repeated name +// wins) against already-evaluated `args`, for a callee whose own parameter +// LIST is `paramNames` (in declared order -- a FunctionDeclaration's +// parameters or a FunctionLiteral's). Shared by CallFn/CallFnTail (a +// site.decl callee) and CallDynamic/CallDynamicTail (a closure callee). +BoundArgs buildBoundArgs(const CompiledChunk::CallSite& site, std::vector& args, size_t argCount, + const std::vector>& paramNames) { + BoundArgs bound; + bound.reserve(argCount); + size_t positionalIdx = 0; + const size_t nparams = paramNames.size(); + for (size_t i = 0; i < argCount; ++i) { + if (site.argNames[i]) { + bound.set(*site.argNames[i], std::move(args[i])); + } else { + if (positionalIdx < nparams) { + bound.set(paramNames[positionalIdx]->name->name, std::move(args[i])); } - case Op::CallDynamicTail: { - const CompiledChunk::CallSite& site = chunk.callSites[static_cast(ins.a)]; - const size_t argCount = static_cast(ins.b); - std::vector args(argCount); - for (size_t i = 0; i < argCount; ++i) { - args[argCount - 1 - i] = std::move(stack.back()); - stack.pop_back(); - } - Value callee = std::move(stack.back()); - stack.pop_back(); - Value result; - if (const auto* closurePtr = std::get_if(&callee); closurePtr && *closurePtr) { - const Closure& closure = **closurePtr; - const oscad::FunctionLiteral& funcNode = *closure.node; - BoundArgs bound; - bound.reserve(argCount); - size_t positionalIdx = 0; - const size_t nparams = funcNode.parameters.size(); - for (size_t i = 0; i < argCount; ++i) { - if (site.argNames[i]) { - bound.set(*site.argNames[i], std::move(args[i])); - } else { - if (positionalIdx < nparams) { - bound.set(funcNode.parameters[positionalIdx]->name->name, std::move(args[i])); - } - ++positionalIdx; - } - } - if (tailOut != nullptr) { - if (auto hopCtx = ev.isolatedCallCtxFor(funcNode, ctx, capturedLetTrail(closure))) { - if (ev.lookupCompiledLiteralChunk(funcNode) != nullptr) { - tailOut->decl = nullptr; - tailOut->literal = &funcNode; - tailOut->bound = std::move(bound); - tailOut->ctx = std::move(*hopCtx); - tailOut->name = ""; - tailOut->callPos = ins.pos; - return Value{}; - } - } - } - result = ev.evalFunctionLiteralFromBound(closure, std::move(bound), ctx, ins.pos); - } else if (!site.calleeName.empty()) { - ev.warn("Ignoring unknown function '" + site.calleeName + "'", ins.pos); - } - stack.push_back(std::move(result)); - ++pc; - break; + ++positionalIdx; + } + } + return bound; +} + +// Binds `bound`'s entries into `frame`'s own slots (matching +// CompiledChunk::Param::name) or, for a declared $-parameter, straight +// into `frame.ctxChain.back().dyn` -- see CompiledChunk::Param::isDyn's +// own doc comment (bytecode.hpp). An undeclared $-named entry is still a +// dynamic-scope override, routed to dyn too; an undeclared plain name has +// no slot and is simply unreferenceable (ponytail: nothing downstream of a +// compiled call ever reads ctx.let_ for a name a compiled body didn't +// itself resolve to a slot, so there's nothing to write there either). +void bindBoundArgsIntoFrame(const CompiledChunk& chunk, const BoundArgs& bound, VmFrame& frame) { + EvalContext& ctx = frame.ctxChain.back(); + for (size_t i = 0; i < chunk.params.size(); ++i) { + const CompiledChunk::Param& p = chunk.params[i]; + if (const Value* v = bound.find(p.name)) { + if (p.isDyn) { + ctx.dyn->set(p.name, *v); + } else { + frame.slots[static_cast(p.slot)] = *v; } - case Op::Echo: { - const CompiledChunk::EchoSite& site = chunk.echoSites[static_cast(ins.a)]; - const size_t argCount = static_cast(ins.b); - std::vector, Value>> pairs(argCount); - for (size_t i = 0; i < argCount; ++i) { - pairs[argCount - 1 - i] = {site.argNames[argCount - 1 - i], std::move(stack.back())}; - stack.pop_back(); - } - ev.emitEcho(pairs); - ++pc; + frame.bound[i] = true; + } + } + for (const auto& [k, v] : bound) { + bool matched = false; + for (const auto& p : chunk.params) { + if (p.name == k) { + matched = true; break; } - case Op::AssertFail: { - // Only ever reached on the condition-false path (guarded by - // a JumpIfTrue the compiler emits around it) -- always - // throws, matching evalAssertExpr's own error() call - // exactly (same message format, same "assert" innermostFrame). - const bool hasMessage = ins.a == 1; - std::string err = "Assertion '" + std::get(chunk.constants[static_cast(ins.b)]) + - "' failed"; - if (hasMessage) { - Value msg = std::move(stack.back()); - stack.pop_back(); - const std::string* s = std::get_if(&msg); - err += ": \"" + (s ? *s : fmtValue(msg)) + "\""; - } - ev.error(err, *ins.node, "assert"); - } } + if (!matched && !k.empty() && k[0] == '$') ctx.dyn->set(k, v); } - return stack.empty() ? Value{} : std::move(stack.back()); } -// 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 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>& arguments, EvalContext& callerCtx, - EvalContext& childCtx, std::vector& slots, std::vector& bound) { +// Same, but for a call site whose arguments are still raw AST (evaluated +// here against `callerCtx`) -- only ever used by runCompiledFunction's own +// entry point (a top-level call with raw AST arguments, mirroring +// evalUserFunction's own bindArgs+bound-loop). The Op::CallFn-family +// opcodes always have already-EVALUATED arguments by the time they resolve +// a callee (popped off the caller frame's own operand stack), so they go +// through buildBoundArgs+bindBoundArgsIntoFrame above instead. +void bindAstArgsIntoFrame(Evaluator& ev, const CompiledChunk& chunk, + const std::vector>& arguments, EvalContext& callerCtx, + VmFrame& frame) { + EvalContext& ctx = frame.ctxChain.back(); size_t positionalIdx = 0; const size_t nparams = chunk.params.size(); for (const auto& argPtr : arguments) { @@ -630,17 +110,17 @@ void bindCompiledArgs(Evaluator& ev, const CompiledChunk& chunk, for (size_t i = 0; i < nparams; ++i) { if (chunk.params[i].name == name) { if (chunk.params[i].isDyn) { - childCtx.dyn->set(name, std::move(v)); + ctx.dyn->set(name, std::move(v)); } else { - slots[static_cast(chunk.params[i].slot)] = std::move(v); + frame.slots[static_cast(chunk.params[i].slot)] = std::move(v); } - bound[i] = true; + frame.bound[i] = true; matched = true; break; } } if (!matched && !name.empty() && name[0] == '$') { - childCtx.dyn->set(name, v); + ctx.dyn->set(name, v); } } else { auto& a = static_cast(*argPtr); @@ -648,185 +128,917 @@ void bindCompiledArgs(Evaluator& ev, const CompiledChunk& chunk, if (positionalIdx < nparams) { const CompiledChunk::Param& p = chunk.params[positionalIdx]; if (p.isDyn) { - childCtx.dyn->set(p.name, std::move(v)); + ctx.dyn->set(p.name, std::move(v)); } else { - slots[static_cast(p.slot)] = std::move(v); + frame.slots[static_cast(p.slot)] = std::move(v); } - bound[positionalIdx] = true; + frame.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) { +Value driveVm(Evaluator& ev, size_t floor); + +// Fills in any parameter left unbound after bindBoundArgsIntoFrame/ +// bindAstArgsIntoFrame from its own default-value expression +// (chunk.defaultCode[i]) -- mirrors applyDefaults' "declaring a parameter +// always creates a fresh binding" rule exactly, same as before this +// redesign. A default's own code is driven through the SAME explicit- +// frame-stack mechanism as everything else (a bare, unbracketed push+ +// drive down to a local floor) rather than a separate execution path -- +// so a call made FROM a default value (rare, but not disallowed by the +// grammar) still gets the full no-native-recursion treatment, not just +// function bodies specifically. +void applyCompiledDefaultsToFrame(Evaluator& ev, const CompiledChunk& chunk, VmFrame& frame) { for (size_t i = 0; i < chunk.params.size(); ++i) { if (frame.bound[i]) continue; const CompiledChunk::Param& p = chunk.params[i]; + Value v; + if (!chunk.defaultCode[i].empty()) { + auto defaultFrame = ev.acquireVmFrame(); + defaultFrame->chunk = &chunk; + defaultFrame->code = &chunk.defaultCode[i]; + defaultFrame->pc = 0; + defaultFrame->slots = frame.slots; // defaults may read earlier SIBLING slots? no -- compiled with an + // isolated (zero-frame) scope, see bytecode_compiler.cpp's + // compileFunctionLike -- copied anyway since Op::LoadLocal + // addresses this SAME chunk's slot numbering and a default's own + // compiled body never references one (isolated scope guarantees + // no LoadLocal survives compilation for a default), so this is + // just "big enough," not "meaningfully shared." + defaultFrame->stack.clear(); + defaultFrame->bound.assign(chunk.params.size(), false); + defaultFrame->accumStack.clear(); + defaultFrame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + defaultFrame->ctxChain.clear(); + defaultFrame->ctxChain.push_back(frame.ctxChain.back()); + defaultFrame->tailHopGuard = 0; + defaultFrame->hopEligible = false; // call-boundary-free, see VmFrame::hopEligible + const size_t floor = ev.vmCallStack_.size(); + ev.vmCallStack_.push_back(std::move(defaultFrame)); + ev.vmCallBrackets_.emplace_back(std::nullopt); + v = driveVm(ev, floor); + } 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(p.slot)] = - runChunk(ev, chunk, chunk.defaultCode[i], frame.slots, childCtx, frame.stack); + frame.ctxChain.back().dyn->set(p.name, std::move(v)); + } else { + frame.slots[static_cast(p.slot)] = std::move(v); } } } -} // namespace +// Constructs (from the pool) and pushes a BARE frame -- no callStack_/ +// profiling bracket, see VmFrame's own doc comment (bytecode_vm.hpp) for +// exactly which call shapes this applies to. +void pushBareFrame(Evaluator& ev, const CompiledChunk& chunk, const std::vector& code, + EvalContext ctx) { + auto frame = ev.acquireVmFrame(); + frame->chunk = &chunk; + frame->code = &code; + frame->pc = 0; + frame->slots.assign(static_cast(chunk.numSlots), Value{}); + frame->stack.clear(); + frame->bound.assign(chunk.params.size(), false); + frame->accumStack.clear(); + frame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + frame->ctxChain.clear(); + frame->ctxChain.push_back(std::move(ctx)); + frame->tailHopGuard = 0; + frame->logicalName.clear(); + frame->hopEligible = false; // both callers (runCompiledExprChunk/ + // runCompiledAssignmentBlock) are genuinely + // call-boundary-free -- see VmFrame::hopEligible. + ev.vmCallStack_.push_back(std::move(frame)); + ev.vmCallBrackets_.emplace_back(std::nullopt); +} -Value runCompiledFunction(Evaluator& ev, const CompiledChunk& chunk, - const std::vector>& arguments, EvalContext& callerCtx, - EvalContext& childCtx, TailCallRequest* tailOut) { - VmFrameGuard guard(ev); - VmFrame& frame = guard.get(); - frame.slots.assign(static_cast(chunk.numSlots), Value{}); - frame.bound.assign(chunk.params.size(), false); - // Stamped onto the current (this call's own) CallStackFrame only once - // `frame.slots` is a well-defined (if still all-undef) array -- a - // nested compiled closure's own LOAD_UPVALUE could in principle find - // this frame from here on; see Evaluator::findUpvalue's own doc - // comment for why that's only reachable once this call's own body - // (which alone could construct/expose such a closure) has actually - // started running, never during argument binding itself. - ev.setCurrentCallVmFrame(&frame); - bindCompiledArgs(ev, chunk, arguments, callerCtx, childCtx, frame.slots, frame.bound); - applyCompiledDefaults(ev, chunk, frame, childCtx); - return runChunk(ev, chunk, chunk.bodyCode, frame.slots, childCtx, frame.stack, tailOut); +// Constructs and pushes a BRACKETED frame -- a genuine new logical call, +// exactly mirroring what evalUserFunctionFromBound/ +// evalFunctionLiteralFromBound used to do via a native recursive call +// (callCtxFor-derived childCtx, upvalueParent computed from +// usedChildCtx, enterUserCall for the callStack_/profiling/checkDebug +// bracket) -- just pushed onto the explicit stack instead of blocking a +// native C++ frame. `declNode`/`bodyExpr` mirror evalUserFunctionCore's +// own parameters exactly (a FunctionDeclaration's decl/*decl.expr, or a +// FunctionLiteral's funcNode/*funcNode.body). +void pushBracketedCallFrame(Evaluator& ev, const CompiledChunk& chunk, const oscad::ASTNode& declNode, + const oscad::Expression& bodyExpr, const std::string& name, BoundArgs bound, + EvalContext& callerCtx, const oscad::Scope* fnScope, + const std::shared_ptr>& capturedLet, const oscad::Position* callPos) { + if (ev.vmCallStack_.size() >= Evaluator::kMaxVmCallStackDepth) { + ev.error("Recursion too deep while calling function '" + name + "'", declNode); + } + const int callerFrameIdx = ev.callStackTopIndex(); + bool usedChildCtx = false; + EvalContext childCtx = ev.callCtxFor(declNode, callerCtx, fnScope, nullptr, nullptr, &usedChildCtx, capturedLet); + const int upvalueParent = usedChildCtx ? callerFrameIdx : -1; + Evaluator::UserCallHandle handle = + ev.enterUserCall(name, declNode, &bodyExpr, childCtx, callPos, upvalueParent, /*skipDepthGuard=*/true); + + auto frame = ev.acquireVmFrame(); + frame->chunk = &chunk; + frame->code = &chunk.bodyCode; + frame->pc = 0; + frame->slots.assign(static_cast(chunk.numSlots), Value{}); + frame->stack.clear(); + frame->bound.assign(chunk.params.size(), false); + frame->accumStack.clear(); + frame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + frame->ctxChain.clear(); + frame->ctxChain.push_back(std::move(childCtx)); + frame->tailHopGuard = 0; + frame->logicalName = name; + frame->hopEligible = true; // just pushed a fresh, correctly-named callStack_ entry + bindBoundArgsIntoFrame(chunk, bound, *frame); + applyCompiledDefaultsToFrame(ev, chunk, *frame); + ev.vmCallStack_.push_back(std::move(frame)); + ev.vmCallBrackets_.push_back(std::move(handle)); } -Value runCompiledFunctionFromBound(Evaluator& ev, const CompiledChunk& chunk, const BoundArgs& bound, - EvalContext& childCtx, TailCallRequest* tailOut) { - VmFrameGuard guard(ev); - VmFrame& frame = guard.get(); - frame.slots.assign(static_cast(chunk.numSlots), Value{}); - frame.bound.assign(chunk.params.size(), false); - ev.setCurrentCallVmFrame(&frame); - for (size_t i = 0; i < chunk.params.size(); ++i) { - 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(p.slot)] = *v; - } - frame.bound[i] = true; - } +// Constructs and pushes a BRACKETED MODULE frame -- the module-side analog +// of pushBracketedCallFrame, used only by Op::CallModule for a NESTED +// module call made from within an already-compiled module body. Unlike a +// function call, there's no BoundArgs/defaults to bind into slots here -- +// `childCtx` arrives already fully prepared (see Evaluator:: +// buildModuleChildCtx, called by Op::CallModule before this runs): a +// module's own parameters were never meant to be slot-addressed in this +// design, only its OWN local variable/for-loop-variable reads inside the +// compiled body go through LoadFree/LoadDyn against ctx, exactly like a +// bare statement-context expression's do. `randsBefore`/`callNode` are +// stashed on the frame itself (moduleRandsBefore/moduleSpliceCallNode) so +// driveVm's own completion branch can run Evaluator::spliceModuleChildren +// once this frame finishes, without needing anywhere else to remember +// them across the (possibly long) run this frame is about to make. +void pushBracketedModuleFrame(Evaluator& ev, const CompiledChunk& chunk, const oscad::ModuleDeclaration& decl, + EvalContext& childCtx, const oscad::Position* callPos, std::uint64_t randsBefore, + const oscad::ASTNode& callNode) { + if (ev.vmCallStack_.size() >= Evaluator::kMaxVmCallStackDepth) { + ev.error("Recursion too deep while calling module '" + decl.name->name + "'", decl); } - for (const auto& [k, v] : bound) { - bool matched = false; - for (const auto& p : chunk.params) { - if (p.name == k) { - matched = true; - break; + Evaluator::UserCallHandle handle = ev.enterUserCall(decl.name->name, decl, /*bodyExpr=*/nullptr, childCtx, callPos, + /*upvalueParent=*/-1, /*skipDepthGuard=*/true, + CallStackFrame::Kind::Module); + auto frame = ev.acquireVmFrame(); + frame->chunk = &chunk; + frame->code = &chunk.bodyCode; + frame->pc = 0; + frame->slots.clear(); + frame->stack.clear(); + frame->bound.clear(); + frame->accumStack.clear(); + frame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + frame->ctxChain.clear(); + frame->ctxChain.push_back(std::move(childCtx)); + frame->tailHopGuard = 0; + frame->logicalName = decl.name->name; + frame->ownsModuleSplice = true; + frame->moduleRandsBefore = randsBefore; + frame->moduleSpliceCallNode = &callNode; + ev.vmCallStack_.push_back(std::move(frame)); + ev.vmCallBrackets_.push_back(std::move(handle)); +} + +// Tears down every frame from vmCallStack_'s own top down to (but not +// including) `floor`, on the exception path -- releases each VmFrame to +// the pool, tears down its ctxChain back-to-front (never relying on +// std::vector's own unspecified destruction order -- see VmFrame's own +// doc comment), and closes out its callStack_/profiling bracket if it had +// one, deliberately WITHOUT firing returnHook (matches +// Evaluator::exitUserCallException's own "only fires on the success path" +// contract). Direct generalization of the pre-redesign ChainTeardown, from +// "one trampoline's own ctx chain" to "the whole explicit call stack." +void teardownVmCallStackDownTo(Evaluator& ev, size_t floor) { + while (ev.vmCallStack_.size() > floor) { + std::unique_ptr frame = std::move(ev.vmCallStack_.back()); + ev.vmCallStack_.pop_back(); + std::optional bracket = std::move(ev.vmCallBrackets_.back()); + ev.vmCallBrackets_.pop_back(); + while (!frame->ctxChain.empty()) frame->ctxChain.pop_back(); + // A module frame's own CALLER (Op::CallModule, mirroring + // evalModularCall/buildTreeNode) always pushes a treeStack_ + // accumulator for it before pushing this frame -- normally popped + // by this same frame's own completion (driveVm's completion + // branch); on the exception path that never runs, so it has to + // happen here instead, exactly like buildTreeNode's own + // catch(...) { treeStack_.pop_back(); throw; }. + if (frame->ownsModuleSplice) ev.treeStack_.pop_back(); + if (bracket) ev.exitUserCallException(*bracket); + ev.releaseVmFrame(std::move(frame)); + } +} + +// The driver -- see this project's own session notes for the full +// "explicit frame stack, not the C++ call stack" rationale. Services +// ev.vmCallStack_ down to (but not including) `floor`, returning the +// value the frame originally AT `floor + 1` (i.e. whichever frame the +// caller just pushed before calling this) produced. Every OTHER frame +// this loop itself pushes (a non-tail Op::CallFn/CallDynamic to another +// compiled function) is fully serviced -- pushed, run, popped, its value +// folded into its own parent's operand stack -- before this function +// returns; only the floor frame's own result escapes to the caller. +Value driveVm(Evaluator& ev, size_t floor) { + Value finalResult; + try { + while (ev.vmCallStack_.size() > floor) { + VmFrame& f = *ev.vmCallStack_.back(); + if (f.pc >= f.code->size()) { + // A module chunk produces nothing on the stack at all -- + // its whole effect already landed in treeStack_ as a side + // effect of running its own body (Op::CallModule/ + // NativeStatement). `result` stays Value{} for one; a + // function chunk's own completion is unchanged. + const bool isModule = f.chunk->isModule; + Value result = (!isModule && !f.stack.empty()) ? std::move(f.stack.back()) : Value{}; + const bool isFloorFrame = ev.vmCallStack_.size() == floor + 1; + std::unique_ptr finished = std::move(ev.vmCallStack_.back()); + ev.vmCallStack_.pop_back(); + std::optional bracket = std::move(ev.vmCallBrackets_.back()); + ev.vmCallBrackets_.pop_back(); + const std::string finishedName = finished->logicalName; + const bool ownsModuleSplice = finished->ownsModuleSplice; + const std::uint64_t moduleRandsBefore = finished->moduleRandsBefore; + const oscad::ASTNode* moduleSpliceCallNode = finished->moduleSpliceCallNode; + while (!finished->ctxChain.empty()) finished->ctxChain.pop_back(); + // Module frames never fire returnHook (native evalUserModule + // never did either -- a module call has no "return value" + // the debugger reports). + if (bracket) ev.exitUserCallSuccess(finishedName, *bracket, result, /*fireReturnHook=*/!isModule); + ev.releaseVmFrame(std::move(finished)); + if (isModule && ownsModuleSplice) { + std::vector> children = std::move(ev.treeStack_.back()); + ev.treeStack_.pop_back(); + ev.spliceModuleChildren(std::move(children), moduleRandsBefore, *moduleSpliceCallNode); + } + if (isFloorFrame) { + finalResult = std::move(result); // unused by a module caller (runCompiledModuleBody ignores it) + break; + } + if (!isModule) ev.vmCallStack_.back()->stack.push_back(std::move(result)); + ++ev.vmCallStack_.back()->pc; + continue; + } + + const Instruction& ins = (*f.code)[f.pc]; + EvalContext& ctx = f.ctxChain.back(); + switch (ins.op) { + case Op::PushConst: + f.stack.push_back(f.chunk->constants[static_cast(ins.a)]); + ++f.pc; + break; + case Op::PushBool: + f.stack.push_back(Value{ins.a != 0}); + ++f.pc; + break; + case Op::LoadLocal: + f.stack.push_back(f.slots[static_cast(ins.a)]); + ++f.pc; + break; + case Op::StoreLocal: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + f.slots[static_cast(ins.a)] = std::move(v); + ++f.pc; + break; + } + case Op::StoreLocalAndLet: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + f.slots[static_cast(ins.a)] = v; + ctx.let_->set(f.chunk->names[static_cast(ins.b)], std::move(v)); + ++f.pc; + break; + } + case Op::LoadDyn: { + const std::string& name = f.chunk->names[static_cast(ins.a)]; + if (const Value* v = ctx.let_->find(name)) { + f.stack.push_back(*v); + } else if (const Value* v = ctx.dyn->find(name)) { + f.stack.push_back(*v); + } else { + f.stack.push_back(Value{}); + } + ++f.pc; + break; + } + case Op::StoreDyn: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + const std::string& name = f.chunk->names[static_cast(ins.a)]; + ctx.dyn->set(name, v); + ctx.dynExplicit->set(name, true); + ++f.pc; + break; + } + case Op::LoadFree: + f.stack.push_back(ev.evalIdentifier(f.chunk->names[static_cast(ins.a)], ins.pos, ctx, true)); + ++f.pc; + break; + case Op::LoadFreeNoWarn: + f.stack.push_back(ev.evalIdentifier(f.chunk->names[static_cast(ins.a)], ins.pos, ctx, false)); + ++f.pc; + break; + case Op::LoadUpvalue: { + const CompiledChunk::UpvalueRef& uv = f.chunk->upvalues[static_cast(ins.a)]; + const Value* v = ev.findUpvalue(uv.targetDecl, uv.slot); + if (!v) v = ctx.let_->find(uv.name); + f.stack.push_back(v ? *v : Value{}); + ++f.pc; + break; + } + case Op::MakeClosure: { + const CompiledChunk::ClosureSite& site = f.chunk->closureSites[static_cast(ins.a)]; + auto capturedTrail = TrailView::makeRoot(); + std::vector selfNames; + for (const auto& cap : site.captures) { + if (cap.isSelfReference) { + selfNames.push_back(&cap.name); + continue; + } + if (cap.targetDecl == f.chunk->selfDecl) { + capturedTrail->set(cap.name, f.slots[static_cast(cap.slot)]); + continue; + } + const Value* v = ev.findUpvalue(cap.targetDecl, cap.slot); + if (!v) v = ctx.let_->find(cap.name); + capturedTrail->set(cap.name, v ? *v : Value{}); + } + auto closure = std::make_shared(Closure{site.node, capturedTrail}); + for (const std::string* selfName : selfNames) { + capturedTrail->set(*selfName, Value{closure}); + } + f.stack.push_back(Value{std::move(closure)}); + ++f.pc; + break; + } + case Op::PatchClosureCapture: { + const Value& consumerVal = f.slots[static_cast(ins.a)]; + if (const auto* closurePtr = std::get_if(&consumerVal); closurePtr && *closurePtr) { + if (auto trail = capturedLetTrail(**closurePtr)) { + trail->set(f.chunk->names[static_cast(ins.b)], f.slots[static_cast(ins.c)]); + } + } + ++f.pc; + break; + } + case Op::Range: { + Value step = std::move(f.stack.back()); + f.stack.pop_back(); + Value end = std::move(f.stack.back()); + f.stack.pop_back(); + Value start = std::move(f.stack.back()); + f.stack.pop_back(); + f.stack.push_back(ev.applyRange(start, end, step)); + ++f.pc; + break; + } + case Op::Index: { + Value idx = std::move(f.stack.back()); + f.stack.pop_back(); + Value obj = std::move(f.stack.back()); + f.stack.pop_back(); + f.stack.push_back(ev.applyIndexAccess(obj, idx)); + ++f.pc; + break; + } + case Op::Member: { + Value obj = std::move(f.stack.back()); + f.stack.pop_back(); + f.stack.push_back(ev.applyMemberAccess(obj, f.chunk->names[static_cast(ins.a)])); + ++f.pc; + break; + } + case Op::UnaryOp: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + f.stack.push_back(ev.applyUnaryOp(static_cast(ins.a), v, *ins.pos)); + ++f.pc; + break; + } + case Op::BinaryOp: { + Value b = std::move(f.stack.back()); + f.stack.pop_back(); + Value a = std::move(f.stack.back()); + f.stack.pop_back(); + f.stack.push_back(ev.applyBinaryOp(static_cast(ins.a), a, b, *ins.pos)); + ++f.pc; + break; + } + case Op::Jump: + f.pc = static_cast(ins.a); + break; + case Op::JumpIfFalse: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + f.pc = !truthy(v) ? static_cast(ins.a) : f.pc + 1; + break; + } + case Op::JumpIfTrue: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + f.pc = truthy(v) ? static_cast(ins.a) : f.pc + 1; + break; + } + case Op::OpenLocalScope: { + for (int i = 0; i < ins.b; ++i) f.slots[static_cast(ins.a + i)] = Value{}; + ++f.pc; + break; + } + case Op::BuildList: { + const size_t count = static_cast(ins.a); + std::vector items(count); + for (size_t i = 0; i < count; ++i) { + items[count - 1 - i] = std::move(f.stack.back()); + f.stack.pop_back(); + } + f.stack.push_back(Value{std::make_shared(ValueList{std::move(items)})}); + ++f.pc; + break; + } + case Op::AccumOpen: + f.accumStack.emplace_back(); + ++f.pc; + break; + case Op::AccumAppendOne: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + f.accumStack.back().push_back(std::move(v)); + ++f.pc; + break; + } + case Op::AccumAppendEach: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + appendEachInto(f.accumStack.back(), v); + ++f.pc; + break; + } + case Op::AccumClose: { + std::vector items = std::move(f.accumStack.back()); + f.accumStack.pop_back(); + f.stack.push_back(Value{std::make_shared(ValueList{std::move(items)})}); + ++f.pc; + break; + } + case Op::AccumMergeEach: { + std::vector inner = std::move(f.accumStack.back()); + f.accumStack.pop_back(); + for (const Value& item : inner) appendEachInto(f.accumStack.back(), item); + ++f.pc; + break; + } + case Op::IterMaterialize: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + IterList& il = f.iterLists[static_cast(ins.a)]; + il.values = expandIterable(v, [&](size_t count) { + ev.warn("Bad range parameter in for statement: too many elements (" + std::to_string(count) + ")", + ins.pos); + }); + il.index = 0; + il.total = il.values.size(); + ++f.pc; + break; + } + case Op::IterReset: { + f.iterLists[static_cast(ins.a)].index = 0; + ++f.pc; + break; + } + case Op::IterNext: { + IterList& il = f.iterLists[static_cast(ins.b)]; + if (il.index < il.total) { + f.slots[static_cast(ins.a)] = il.values[il.index]; + ++il.index; + ++f.pc; + } else { + f.pc = static_cast(ins.c); + } + break; + } + case Op::CheckIterLimit: { + double count = std::get(f.slots[static_cast(ins.a)]) + 1.0; + f.slots[static_cast(ins.a)] = Value{count}; + if (count > static_cast(ins.b)) { + ev.error("C-style for loop exceeded maximum iteration count", *ins.node); + } + ++f.pc; + break; + } + case Op::CallFn: { + const CompiledChunk::CallSite& site = f.chunk->callSites[static_cast(ins.a)]; + const size_t argCount = static_cast(ins.b); + std::vector args(argCount); + for (size_t i = 0; i < argCount; ++i) { + args[argCount - 1 - i] = std::move(f.stack.back()); + f.stack.pop_back(); + } + if (site.isImport) { + CallArgs callArgs = buildCallArgs(site, args, argCount); + f.stack.push_back(importAsValue(ev, callArgs, *site.callNode)); + ++f.pc; + } else if (site.isBuiltin && site.calleeName == "object") { + std::vector, Value>> pairs; + pairs.reserve(argCount); + for (size_t i = 0; i < argCount; ++i) pairs.emplace_back(site.argNames[i], std::move(args[i])); + f.stack.push_back(mergeObjectArgs(pairs)); + ++f.pc; + } else if (site.isBuiltin) { + CallArgs callArgs = buildCallArgs(site, args, argCount); + f.stack.push_back(evalBuiltinFunction(ev, site.calleeName, callArgs, *site.callNode)); + ++f.pc; + } else { + BoundArgs bound = buildBoundArgs(site, args, argCount, site.decl->parameters); + const CompiledChunk* calleeChunk = ev.useBytecodeVm() ? ev.lookupOrCompileChunk(*site.decl) : nullptr; + if (calleeChunk) { + const oscad::Scope* fnScope = site.decl->scope() ? site.decl->scope() : ctx.scope; + pushBracketedCallFrame(ev, *calleeChunk, *site.decl, *site.decl->expr, site.calleeName, + std::move(bound), ctx, fnScope, nullptr, &site.callNode->position()); + // f.pc deliberately NOT advanced -- resumes when the + // pushed frame completes (see driveVm's own + // completion branch, above). + } else { + Value result = ev.evalUserFunctionFromBound(site.calleeName, *site.decl, std::move(bound), ctx, + &site.callNode->position()); + f.stack.push_back(std::move(result)); + ++f.pc; + } + } + break; + } + case Op::CallDynamic: { + const CompiledChunk::CallSite& site = f.chunk->callSites[static_cast(ins.a)]; + const size_t argCount = static_cast(ins.b); + std::vector args(argCount); + for (size_t i = 0; i < argCount; ++i) { + args[argCount - 1 - i] = std::move(f.stack.back()); + f.stack.pop_back(); + } + Value callee = std::move(f.stack.back()); + f.stack.pop_back(); + if (const auto* closurePtr = std::get_if(&callee); closurePtr && *closurePtr) { + const Closure& closure = **closurePtr; + const oscad::FunctionLiteral& funcNode = *closure.node; + BoundArgs bound = buildBoundArgs(site, args, argCount, funcNode.parameters); + const CompiledChunk* calleeChunk = ev.useBytecodeVm() ? ev.lookupCompiledLiteralChunk(funcNode) : nullptr; + if (calleeChunk) { + const oscad::Scope* fnScope = funcNode.scope() ? funcNode.scope() : ctx.scope; + pushBracketedCallFrame(ev, *calleeChunk, funcNode, *funcNode.body, "", + std::move(bound), ctx, fnScope, capturedLetTrail(closure), ins.pos); + } else { + Value result = ev.evalFunctionLiteralFromBound(closure, std::move(bound), ctx, ins.pos); + f.stack.push_back(std::move(result)); + ++f.pc; + } + } else { + if (!site.calleeName.empty()) ev.warn("Ignoring unknown function '" + site.calleeName + "'", ins.pos); + f.stack.push_back(Value{}); + ++f.pc; + } + break; + } + case Op::CallFnTail: { + const CompiledChunk::CallSite& site = f.chunk->callSites[static_cast(ins.a)]; + const size_t argCount = static_cast(ins.b); + std::vector args(argCount); + for (size_t i = 0; i < argCount; ++i) { + args[argCount - 1 - i] = std::move(f.stack.back()); + f.stack.pop_back(); + } + BoundArgs bound = buildBoundArgs(site, args, argCount, site.decl->parameters); + // Tail-hop-in-place requires f.hopEligible -- a frame + // that's call-boundary-free (statement expression, + // assignment block, parameter default) has no + // callStack_ entry that's genuinely its own, so + // recordTailCallHop below (which mutates + // callStack_.back()) would corrupt whatever UNRELATED + // call happens to be on top of callStack_ instead -- + // see VmFrame::hopEligible's own doc comment for why + // this is NOT the same test as "did THIS frame push its + // own vmCallBrackets_ entry" (runCompiledFunction's + // initial frame is bare in that sense but still + // hop-eligible). Falls through to the exact same + // push-or-native-fallback behavior as Op::CallFn in that + // case -- still gets the full explicit-stack treatment if + // the callee compiles, just as a genuine new logical call + // rather than a hop, mirroring today's `tailOut==nullptr` + // behavior for these same bare call shapes exactly. + const bool thisFrameBracketed = f.hopEligible; + std::optional hopCtx = + thisFrameBracketed ? ev.isolatedCallCtxFor(*site.decl, ctx) : std::nullopt; + const CompiledChunk* calleeChunk = + (thisFrameBracketed && hopCtx && ev.useBytecodeVm()) ? ev.lookupOrCompileChunk(*site.decl) : nullptr; + if (calleeChunk) { + ev.recordTailCallHop(site.calleeName, *site.decl, &site.callNode->position(), f.tailHopGuard); + f.chunk = calleeChunk; + f.code = &calleeChunk->bodyCode; + f.pc = 0; + f.slots.assign(static_cast(calleeChunk->numSlots), Value{}); + f.stack.clear(); + f.bound.assign(calleeChunk->params.size(), false); + f.accumStack.clear(); + f.iterLists.assign(static_cast(calleeChunk->numIterLists), IterList{}); + f.ctxChain.push_back(std::move(*hopCtx)); + bindBoundArgsIntoFrame(*calleeChunk, bound, f); + applyCompiledDefaultsToFrame(ev, *calleeChunk, f); + // continue via the outer while -- re-fetches this + // same (now-mutated) frame from its new pc=0. + } else { + const CompiledChunk* fallbackChunk = + ev.useBytecodeVm() ? ev.lookupOrCompileChunk(*site.decl) : nullptr; + if (fallbackChunk) { + const oscad::Scope* fnScope = site.decl->scope() ? site.decl->scope() : ctx.scope; + pushBracketedCallFrame(ev, *fallbackChunk, *site.decl, *site.decl->expr, site.calleeName, + std::move(bound), ctx, fnScope, nullptr, &site.callNode->position()); + } else { + Value result = ev.evalUserFunctionFromBound(site.calleeName, *site.decl, std::move(bound), ctx, + &site.callNode->position()); + f.stack.push_back(std::move(result)); + ++f.pc; + } + } + break; + } + case Op::CallDynamicTail: { + const CompiledChunk::CallSite& site = f.chunk->callSites[static_cast(ins.a)]; + const size_t argCount = static_cast(ins.b); + std::vector args(argCount); + for (size_t i = 0; i < argCount; ++i) { + args[argCount - 1 - i] = std::move(f.stack.back()); + f.stack.pop_back(); + } + Value callee = std::move(f.stack.back()); + f.stack.pop_back(); + if (const auto* closurePtr = std::get_if(&callee); closurePtr && *closurePtr) { + const Closure& closure = **closurePtr; + const oscad::FunctionLiteral& funcNode = *closure.node; + BoundArgs bound = buildBoundArgs(site, args, argCount, funcNode.parameters); + const bool thisFrameBracketed = f.hopEligible; + std::optional hopCtx = thisFrameBracketed + ? ev.isolatedCallCtxFor(funcNode, ctx, capturedLetTrail(closure)) + : std::nullopt; + const CompiledChunk* calleeChunk = (thisFrameBracketed && hopCtx && ev.useBytecodeVm()) + ? ev.lookupCompiledLiteralChunk(funcNode) + : nullptr; + if (calleeChunk) { + ev.recordTailCallHop("", funcNode, ins.pos, f.tailHopGuard); + f.chunk = calleeChunk; + f.code = &calleeChunk->bodyCode; + f.pc = 0; + f.slots.assign(static_cast(calleeChunk->numSlots), Value{}); + f.stack.clear(); + f.bound.assign(calleeChunk->params.size(), false); + f.accumStack.clear(); + f.iterLists.assign(static_cast(calleeChunk->numIterLists), IterList{}); + f.ctxChain.push_back(std::move(*hopCtx)); + bindBoundArgsIntoFrame(*calleeChunk, bound, f); + applyCompiledDefaultsToFrame(ev, *calleeChunk, f); + } else { + const CompiledChunk* fallbackChunk = + ev.useBytecodeVm() ? ev.lookupCompiledLiteralChunk(funcNode) : nullptr; + if (fallbackChunk) { + const oscad::Scope* fnScope = funcNode.scope() ? funcNode.scope() : ctx.scope; + pushBracketedCallFrame(ev, *fallbackChunk, funcNode, *funcNode.body, "", + std::move(bound), ctx, fnScope, capturedLetTrail(closure), ins.pos); + } else { + Value result = ev.evalFunctionLiteralFromBound(closure, std::move(bound), ctx, ins.pos); + f.stack.push_back(std::move(result)); + ++f.pc; + } + } + } else { + if (!site.calleeName.empty()) ev.warn("Ignoring unknown function '" + site.calleeName + "'", ins.pos); + f.stack.push_back(Value{}); + ++f.pc; + } + break; + } + case Op::Echo: { + const CompiledChunk::EchoSite& site = f.chunk->echoSites[static_cast(ins.a)]; + const size_t argCount = static_cast(ins.b); + std::vector, Value>> pairs(argCount); + for (size_t i = 0; i < argCount; ++i) { + pairs[argCount - 1 - i] = {site.argNames[argCount - 1 - i], std::move(f.stack.back())}; + f.stack.pop_back(); + } + ev.emitEcho(pairs); + ++f.pc; + break; + } + case Op::AssertFail: { + const bool hasMessage = ins.a == 1; + std::string err = + "Assertion '" + std::get(f.chunk->constants[static_cast(ins.b)]) + "' failed"; + if (hasMessage) { + Value msg = std::move(f.stack.back()); + f.stack.pop_back(); + const std::string* s = std::get_if(&msg); + err += ": \"" + (s ? *s : fmtValue(msg)) + "\""; + } + ev.error(err, *ins.node, "assert"); + } + case Op::CallModule: { + const CompiledChunk::ModuleCallSite& site = f.chunk->moduleCallSites[static_cast(ins.a)]; + BoundArgs bound = ev.bindArgs(site.decl->parameters, site.callNode->arguments, ctx); + EvalContext childCtx = ev.buildModuleChildCtx(*site.decl, *site.callNode, ctx, std::move(bound)); + ev.treeStack_.emplace_back(); + const std::uint64_t randsBefore = ev.randsCallCount(); + const CompiledChunk* calleeChunk = ev.useBytecodeVm() ? ev.lookupOrCompileModuleChunk(*site.decl) : nullptr; + if (calleeChunk) { + pushBracketedModuleFrame(ev, *calleeChunk, *site.decl, childCtx, &site.callNode->position(), + randsBefore, *site.callNode); + // f.pc deliberately NOT advanced -- resumes when the + // pushed frame completes (see driveVm's own module + // completion branch, above). + } else { + ev.runModuleBodyNative(*site.decl, childCtx, &site.callNode->position()); + std::vector> children = std::move(ev.treeStack_.back()); + ev.treeStack_.pop_back(); + ev.spliceModuleChildren(std::move(children), randsBefore, *site.callNode); + ++f.pc; + } + break; + } + case Op::NativeStatement: { + const oscad::ASTNode* stmt = f.chunk->nativeStatements[static_cast(ins.a)]; + EvalContext childCtx = ctx.withScope(stmt->scope() ? stmt->scope() : ctx.scope); + // Mirrors evalChildren's own per-statement loop exactly, + // including its ModularLet exclusion (evalLetBlock fires + // its own per-assignment checkDebug internally). + if (stmt->kind() != oscad::NodeKind::ModularLet) ev.checkDebug(*stmt, childCtx); + ev.evalStatement(*stmt, childCtx); + ++f.pc; + break; + } + case Op::NativeCondJumpIfFalse: { + const oscad::Expression* cond = f.chunk->nativeExprs[static_cast(ins.a)]; + const bool truthyVal = truthy(ev.evalExprMaybeCompiled(*cond, ctx)); + f.pc = truthyVal ? f.pc + 1 : static_cast(ins.b); + break; + } + case Op::NativeCheckDebugExprLevel: { + const oscad::ASTNode* marker = f.chunk->nativeStatements[static_cast(ins.a)]; + ev.checkDebug(*marker, ctx, /*forced=*/false, /*exprLevel=*/true); + ++f.pc; + break; + } + case Op::NativeIterMaterialize: { + const oscad::Expression* rangeExpr = f.chunk->nativeExprs[static_cast(ins.a)]; + Value v = ev.evalExprMaybeCompiled(*rangeExpr, ctx); + IterList& il = f.iterLists[static_cast(ins.b)]; + il.values = expandIterable(v, [&](size_t count) { + ev.warn("Bad range parameter in for statement: too many elements (" + std::to_string(count) + ")", + ins.pos); + }); + il.index = 0; + il.total = il.values.size(); + ++f.pc; + break; + } + case Op::ForIterNext: { + IterList& il = f.iterLists[static_cast(ins.b)]; + if (il.index < il.total) { + // Fresh child ctx per iteration -- mirrors evalFor's + // own parentCtx.childCtx(...) exactly (see + // Op::ForIterNext's own doc comment, bytecode.hpp, + // for why reusing the SAME ctx across iterations + // would be observably wrong, not just slower). + EvalContext iterCtx = ctx.childCtx(nullptr, std::nullopt, ctx.childrenNodes, ctx.childrenCallerCtx); + iterCtx.let_->set(f.chunk->names[static_cast(ins.a)], il.values[il.index]); + ++il.index; + f.ctxChain.push_back(std::move(iterCtx)); + ev.checkDebug(*ins.node, f.ctxChain.back()); + ++f.pc; + } else { + f.pc = static_cast(ins.c); + } + break; + } + case Op::ForIterEnd: { + f.ctxChain.pop_back(); + f.pc = static_cast(ins.a); + break; + } } } - if (!matched && !k.empty() && k[0] == '$') childCtx.dyn->set(k, v); + } catch (...) { + teardownVmCallStackDownTo(ev, floor); + throw; } - applyCompiledDefaults(ev, chunk, frame, childCtx); - return runChunk(ev, chunk, chunk.bodyCode, frame.slots, childCtx, frame.stack, tailOut); + return finalResult; } -// The trampoline entry points -- see bytecode_vm.hpp's own doc comment for -// the overview. Both share the identical loop shape: run the first -// (real) call via the ordinary entry function (with tailOut populated), -// then keep reusing runCompiledFunctionFromBound directly for every -// subsequent hop (a TailCallRequest's own bound-args shape already -// matches that function's own parameter shape exactly -- no new binding -// logic needed). -// -// Every hop's own EvalContext (TailCallRequest::ctx, already derived by -// isolatedCallCtxFor -- a fresh $-var/dyn trail level) is kept alive for -// the WHOLE trampoline run via `chain`, mirroring evalFunctionBodyTrampoline's -// identical fix on the interpreter side (user_calls.cpp) exactly: $-vars -// stay dynamically scoped THROUGH even an isolated call (callCtx()'s own -// dyn uses isolate=false), so a later hop may still need to walk back -// through a much earlier one's dyn level to resolve a name. Reusing a -// single EvalContext variable via plain assignment would drop an earlier -// level's shared_ptr refcount to zero and pop it (ScopeTrailStorage:: -// popLevel, scope_trail.hpp's custom-deleter mechanism) -- permanently -// erasing a parent-chain link a later hop still needs. Trades native C++ -// *stack* growth (what this trampoline exists to eliminate) for heap -// growth instead -- O(iteration count) EvalContext objects, bounded by -// available RAM rather than a ~few-MB thread stack. -namespace { -// Tears a trampoline's own `chain` down back-to-front on EVERY exit path -- -// normal return OR exception unwind (recordTailCallHop's own recursion-guard -// error is the exact case that matters: an infinite tail-recursive function -// with no base case throws out of the loop entirely, bypassing any teardown -// code placed after it). Relying on `chain` simply falling out of scope -// instead is NOT equivalent: std::vector's own element-destruction order -// is unspecified by the standard -- libc++ destroys back-to-front (matching -// ScopeTrailStorage::popLevel's own "scan from the back" doc comment, which -// assumes the level being popped is usually the most recently pushed one -- -// O(1) per pop that way), but libstdc++ destroys front-to-back, the -// adversarial order for that same scan -- each pop then walks almost the -// entire remaining vector, turning an intended O(N) teardown into a real, -// measured O(N^2) (see issue #50: an N-scaling experiment plus a minimal -// repro proving libc++/libstdc++ disagree on std::vector::clear()'s -// destruction order for identical source). Popping back-to-front explicitly, -// via a destructor that always runs before `chain`'s own, is correct AND -// O(N) on every standard library and every exit path, not just the ones -// that happen to agree with libc++ and return normally. -struct ChainTeardown { - std::vector& chain; - ~ChainTeardown() { - while (!chain.empty()) chain.pop_back(); - } -}; } // namespace -Value runCompiledFunctionTrampoline(Evaluator& ev, const CompiledChunk& chunk, - const std::vector>& arguments, - EvalContext& callerCtx, EvalContext& childCtx) { - std::vector chain{childCtx}; - ChainTeardown teardown{chain}; - TailCallRequest req; - Value result = runCompiledFunction(ev, chunk, arguments, callerCtx, chain.back(), &req); - unsigned recursionGuard = 0; - while (req.decl != nullptr || req.literal != nullptr) { - const oscad::ASTNode& calleeDecl = - req.decl ? static_cast(*req.decl) : static_cast(*req.literal); - ev.recordTailCallHop(req.name, calleeDecl, req.callPos, recursionGuard); - const CompiledChunk* curChunk = - req.decl ? ev.lookupOrCompileChunk(*req.decl) : ev.lookupCompiledLiteralChunk(*req.literal); - BoundArgs bound = std::move(req.bound); - chain.push_back(std::move(req.ctx)); - TailCallRequest next; - result = runCompiledFunctionFromBound(ev, *curChunk, bound, chain.back(), &next); - req = std::move(next); - } - return result; +Value runCompiledFunction(Evaluator& ev, const CompiledChunk& chunk, + const std::vector>& arguments, EvalContext& callerCtx, + EvalContext& childCtx) { + auto frame = ev.acquireVmFrame(); + frame->chunk = &chunk; + frame->code = &chunk.bodyCode; + frame->pc = 0; + frame->slots.assign(static_cast(chunk.numSlots), Value{}); + frame->stack.clear(); + frame->bound.assign(chunk.params.size(), false); + frame->accumStack.clear(); + frame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + frame->ctxChain.clear(); + frame->ctxChain.push_back(childCtx); + frame->tailHopGuard = 0; + frame->logicalName.clear(); + frame->hopEligible = true; // caller's own enterUserCall already made + // callStack_.back() this call -- see + // VmFrame::hopEligible. + bindAstArgsIntoFrame(ev, chunk, arguments, callerCtx, *frame); + applyCompiledDefaultsToFrame(ev, chunk, *frame); + const size_t floor = ev.vmCallStack_.size(); + ev.vmCallStack_.push_back(std::move(frame)); + ev.vmCallBrackets_.emplace_back(std::nullopt); + return driveVm(ev, floor); } -Value runCompiledFunctionFromBoundTrampoline(Evaluator& ev, const CompiledChunk& chunk, const BoundArgs& bound, - EvalContext& childCtx) { - std::vector chain{childCtx}; - ChainTeardown teardown{chain}; - TailCallRequest req; - Value result = runCompiledFunctionFromBound(ev, chunk, bound, chain.back(), &req); - unsigned recursionGuard = 0; - while (req.decl != nullptr || req.literal != nullptr) { - const oscad::ASTNode& calleeDecl = - req.decl ? static_cast(*req.decl) : static_cast(*req.literal); - ev.recordTailCallHop(req.name, calleeDecl, req.callPos, recursionGuard); - const CompiledChunk* curChunk = - req.decl ? ev.lookupOrCompileChunk(*req.decl) : ev.lookupCompiledLiteralChunk(*req.literal); - BoundArgs nextBound = std::move(req.bound); - chain.push_back(std::move(req.ctx)); - TailCallRequest next; - result = runCompiledFunctionFromBound(ev, *curChunk, nextBound, chain.back(), &next); - req = std::move(next); - } - return result; +Value runCompiledFunctionFromBound(Evaluator& ev, const CompiledChunk& chunk, const BoundArgs& bound, + EvalContext& childCtx) { + auto frame = ev.acquireVmFrame(); + frame->chunk = &chunk; + frame->code = &chunk.bodyCode; + frame->pc = 0; + frame->slots.assign(static_cast(chunk.numSlots), Value{}); + frame->stack.clear(); + frame->bound.assign(chunk.params.size(), false); + frame->accumStack.clear(); + frame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + frame->ctxChain.clear(); + frame->ctxChain.push_back(childCtx); + frame->tailHopGuard = 0; + frame->logicalName.clear(); + frame->hopEligible = true; // caller's own enterUserCall already made + // callStack_.back() this call -- see + // VmFrame::hopEligible. + bindBoundArgsIntoFrame(chunk, bound, *frame); + applyCompiledDefaultsToFrame(ev, chunk, *frame); + const size_t floor = ev.vmCallStack_.size(); + ev.vmCallStack_.push_back(std::move(frame)); + ev.vmCallBrackets_.emplace_back(std::nullopt); + return driveVm(ev, floor); +} + +Value runCompiledExprChunk(Evaluator& ev, const CompiledChunk& chunk, EvalContext& ctx) { + // A fresh letChildCtx(), not `ctx` directly -- Op::OpenLocalScope (a + // nested let()'s own compiled entry) only resets SLOTS, never opens a + // new ctx.dyn trail level: a `let($fn=...)` inside the expression + // writes straight into whatever dyn level is current. That's harmless + // for a genuine call frame (each of THOSE already derives its own + // fresh, call-scoped childCtx via callCtxFor), but this wrapper is + // handed the CALLER's own ctx directly (a statement-context expression + // runs in the same scope as its statement, not a new call boundary), + // so without this, a `$fn` override would leak into every statement + // sharing `ctx` AFTER this one. Caught for real this session: + // `v1 = let($fn=55) f(); v2 = f();` returned 55 for v2 too. + const size_t floor = ev.vmCallStack_.size(); + pushBareFrame(ev, chunk, chunk.bodyCode, ctx.letChildCtx()); + return driveVm(ev, floor); +} + +void runCompiledAssignmentBlock(Evaluator& ev, const CompiledChunk& chunk, EvalContext& ctx) { + const size_t floor = ev.vmCallStack_.size(); + // `ctx` directly -- Op::StoreLocalAndLet/Op::StoreDyn's own writes must + // persist into the caller's own scope; see tryCompileAssignmentBlock's + // own doc comment (bytecode_compiler.hpp) for why a nested let()'s own + // dyn-scoping hazard doesn't apply here (refused at compile time + // instead of needing runtime containment). + pushBareFrame(ev, chunk, chunk.bodyCode, ctx); + driveVm(ev, floor); +} + +void runCompiledModuleBody(Evaluator& ev, const CompiledChunk& chunk, EvalContext& childCtx) { + // Bare push -- unlike pushBracketedModuleFrame, this frame's own + // splice responsibility belongs to whichever NATIVE evalModularCall + // called Evaluator::evalUserModule in the first place (it already + // pushed treeStack_'s own accumulator before calling in here) -- + // mirrors runCompiledFunction's own bare/bracket-owned-by-caller split + // exactly (VmFrame::ownsModuleSplice's own doc comment, bytecode_vm.hpp). + auto frame = ev.acquireVmFrame(); + frame->chunk = &chunk; + frame->code = &chunk.bodyCode; + frame->pc = 0; + frame->slots.clear(); + frame->stack.clear(); + frame->bound.clear(); + frame->accumStack.clear(); + frame->iterLists.assign(static_cast(chunk.numIterLists), IterList{}); + frame->ctxChain.clear(); + frame->ctxChain.push_back(childCtx); + frame->tailHopGuard = 0; + frame->logicalName.clear(); + frame->ownsModuleSplice = false; + frame->moduleRandsBefore = 0; + frame->moduleSpliceCallNode = nullptr; + const size_t floor = ev.vmCallStack_.size(); + ev.vmCallStack_.push_back(std::move(frame)); + ev.vmCallBrackets_.emplace_back(std::nullopt); + driveVm(ev, floor); } } // namespace oscadeval diff --git a/src/csg_resolve.cpp b/src/csg_resolve.cpp index cc90dca..eaf6870 100644 --- a/src/csg_resolve.cpp +++ b/src/csg_resolve.cpp @@ -10,6 +10,24 @@ namespace oscadeval { +namespace { +// Scopes Evaluator::inResolvePass_ to one resolveTreeImpl call, restored on +// every exit path including an exception thrown from evalChildren (an +// assert() failure, mid-tree) -- see stmtExprChunkCache_'s own doc comment +// (evaluator.hpp) for why evalExprMaybeCompiled must never touch its cache +// outside an active pass. +class ResolvePassGuard { +public: + explicit ResolvePassGuard(bool& flag) : flag_(flag) { flag_ = true; } + ~ResolvePassGuard() { flag_ = false; } + ResolvePassGuard(const ResolvePassGuard&) = delete; + ResolvePassGuard& operator=(const ResolvePassGuard&) = delete; + +private: + bool& flag_; +}; +} // namespace + void Evaluator::buildTreeNode(const std::string& kind, const oscad::ASTNode& node, const std::function& resolveBody) { // Push a fresh accumulator for whatever CSGNodes get created while @@ -97,27 +115,7 @@ void Evaluator::evalModularCall(const oscad::ModularCall& node, EvalContext& ctx // _eval_statement's splice branch exactly. const bool splice = (name == "children" && isBuiltin) || !isBuiltin; if (splice) { - if (randsCallCount_ != randsBefore) { - // rands() fired directly during *this* call's own resolve - // (e.g. an assignment before any geometry statement in a user - // module's body) rather than inside one of the spliced - // children's own resolve (which already self-taints via the - // branch below) -- propagate onto every spliced child so the - // taint isn't silently dropped by splicing away the node it - // would otherwise have landed on. - for (auto& c : children) c->uncacheable = true; - } - if (children.size() > 1) { - auto unionNode = std::make_unique(); - unionNode->kind = "union"; - unionNode->node = &node; - unionNode->isBuiltin = false; - unionNode->uncacheable = std::any_of(children.begin(), children.end(), [](const auto& c) { return c->uncacheable; }); - unionNode->children = std::move(children); - treeStack_.back().push_back(std::move(unionNode)); - } else { - for (auto& c : children) treeStack_.back().push_back(std::move(c)); - } + spliceModuleChildren(std::move(children), randsBefore, node); return; } @@ -133,6 +131,31 @@ void Evaluator::evalModularCall(const oscad::ModularCall& node, EvalContext& ctx treeStack_.back().push_back(std::move(treeNode)); } +void Evaluator::spliceModuleChildren(std::vector> children, std::uint64_t randsBefore, + const oscad::ASTNode& callNode) { + if (randsCallCount_ != randsBefore) { + // rands() fired directly during *this* call's own resolve (e.g. an + // assignment before any geometry statement in a user module's + // body) rather than inside one of the spliced children's own + // resolve (which already self-taints via the branch below) -- + // propagate onto every spliced child so the taint isn't silently + // dropped by splicing away the node it would otherwise have + // landed on. + for (auto& c : children) c->uncacheable = true; + } + if (children.size() > 1) { + auto unionNode = std::make_unique(); + unionNode->kind = "union"; + unionNode->node = &callNode; + unionNode->isBuiltin = false; + unionNode->uncacheable = std::any_of(children.begin(), children.end(), [](const auto& c) { return c->uncacheable; }); + unionNode->children = std::move(children); + treeStack_.back().push_back(std::move(unionNode)); + } else { + for (auto& c : children) treeStack_.back().push_back(std::move(c)); + } +} + void Evaluator::evalModifier(const oscad::ModuleInstantiation& child, const oscad::ASTNode& wrapperNode, const std::string& kind, EvalContext& ctx) { // Mirrors _resolve_modifier_child: evaluate the single wrapped child @@ -167,6 +190,15 @@ std::vector> Evaluator::resolveTreeImpl(const NodeList& treeStack_.emplace_back(); rootCtx_ = &ctx; lastCtx_ = &ctx; + // Cleared here, not just guarded by inResolvePass_ below, so a host + // that reuses one Evaluator across separate top-level resolveTree()/ + // evaluate() calls (each against a freshly re-parsed AST -- the normal + // "re-render after an edit" pattern) never risks a stale entry from a + // PRIOR pass whose own AST nodes may since have been freed. See + // stmtExprChunkCache_'s own doc comment (evaluator.hpp) for the full + // hazard this guards against. + stmtExprChunkCache_.clear(); + ResolvePassGuard resolvePassGuard(inResolvePass_); evalChildren(nodes, ctx); std::vector> tree = std::move(treeStack_.back()); treeStack_.pop_back(); diff --git a/src/stmt_eval.cpp b/src/stmt_eval.cpp index 007709a..66a149a 100644 --- a/src/stmt_eval.cpp +++ b/src/stmt_eval.cpp @@ -9,7 +9,7 @@ namespace oscadeval { void Evaluator::evalAssignment(const oscad::Assignment& node, EvalContext& ctx) { const std::string& name = node.name->name; if (!name.empty() && name[0] == '$') { - ctx.dyn->set(name, evalExpr(*node.expr, ctx)); + ctx.dyn->set(name, evalExprMaybeCompiled(*node.expr, ctx)); ctx.dynExplicit->set(name, true); return; } @@ -20,7 +20,7 @@ void Evaluator::evalAssignment(const oscad::Assignment& node, EvalContext& ctx) const int firstLine = firstPos ? firstPos->line : 0; warn(name + " was assigned on line " + std::to_string(firstLine) + " but was overwritten", pos); } - ctx.let_->set(name, evalExpr(*node.expr, ctx)); + ctx.let_->set(name, evalExprMaybeCompiled(*node.expr, ctx)); ctx.dynPositions->set(name, pos); } @@ -39,7 +39,7 @@ void Evaluator::doEcho(const std::vector>& argu pairs.reserve(arguments.size()); for (const auto& argPtr : arguments) { const oscad::Argument& arg = *argPtr; - Value val = evalExpr(*argExpr(arg), ctx); + Value val = evalExprMaybeCompiled(*argExpr(arg), ctx); std::optional name; if (arg.kind() == oscad::NodeKind::NamedArgument) name = static_cast(arg).name->name; pairs.emplace_back(std::move(name), std::move(val)); @@ -78,7 +78,7 @@ void Evaluator::evalFor(const oscad::ModularFor& node, EvalContext& ctx) { std::vector pairs; pairs.reserve(node.assignments.size()); for (const auto& assign : node.assignments) { - Value values = evalExpr(*assign->expr, ctx); + Value values = evalExprMaybeCompiled(*assign->expr, ctx); const oscad::Position* pos = &assign->position(); pairs.push_back(AssignPair{assign->name->name, expandIterable(values, [&](size_t count) { warn("Bad range parameter in for statement: too many elements (" + std::to_string(count) + ")", pos); @@ -135,7 +135,7 @@ void Evaluator::evalLetBlock(const oscad::ModularLet& node, EvalContext& ctx) { // Checked against the ORIGINAL ctx (like the RHS eval below), not // childCtx -- mirrors _eval_let_block's `_check_debug(assign, ctx)`. checkDebug(*assign, ctx); - Value v = evalExpr(*assign->expr, ctx); + Value v = evalExprMaybeCompiled(*assign->expr, ctx); const std::string& name = assign->name->name; if (!name.empty() && name[0] == '$') { childCtx.dyn->set(name, v); @@ -186,7 +186,7 @@ void Evaluator::evalStatement(const oscad::ASTNode& node, EvalContext& ctx) { // `_check_debug(branch[0] if branch else node, ctx, expr_level=True)`. case oscad::NodeKind::ModularIf: { auto& n = static_cast(node); - if (truthy(evalExpr(*n.condition, ctx))) { + if (truthy(evalExprMaybeCompiled(*n.condition, ctx))) { checkDebug(n.trueBranch.empty() ? node : *n.trueBranch.front(), ctx, /*forced=*/false, /*exprLevel=*/true); evalChildren(n.trueBranch, ctx); @@ -195,7 +195,7 @@ void Evaluator::evalStatement(const oscad::ASTNode& node, EvalContext& ctx) { } case oscad::NodeKind::ModularIfElse: { auto& n = static_cast(node); - const auto& branch = truthy(evalExpr(*n.condition, ctx)) ? n.trueBranch : n.falseBranch; + const auto& branch = truthy(evalExprMaybeCompiled(*n.condition, ctx)) ? n.trueBranch : n.falseBranch; checkDebug(branch.empty() ? node : *branch.front(), ctx, /*forced=*/false, /*exprLevel=*/true); evalChildren(branch, ctx); return; @@ -262,7 +262,14 @@ void Evaluator::evalChildren(const std::vector& children, evalStatement(*child, childCtx); } }; - runAll(assignments); + // Whole-batch compile first (see tryRunCompiledAssignmentBlock's own + // doc comment, evaluator.hpp) -- falls back to the ordinary per- + // statement loop whenever compiling/running the batch that way isn't + // possible or eligible, unconditionally correct either way since + // nothing about these assignments' observable behavior differs based + // on which path ran them. + lastCtx_ = &ctx; + if (!tryRunCompiledAssignmentBlock(assignments, ctx)) runAll(assignments); runAll(others); } diff --git a/src/user_calls.cpp b/src/user_calls.cpp index d60380d..1a5b1b2 100644 --- a/src/user_calls.cpp +++ b/src/user_calls.cpp @@ -30,6 +30,16 @@ const CompiledChunk* Evaluator::lookupCompiledLiteralChunk(const oscad::Function return chunkEligibleNow(it->second) ? &it->second : nullptr; } +const CompiledChunk* Evaluator::lookupOrCompileModuleChunk(const oscad::ModuleDeclaration& decl) { + auto it = moduleChunkCache_.find(&decl); + if (it == moduleChunkCache_.end()) { + it = moduleChunkCache_.emplace(&decl, tryCompileModuleBody(decl)).first; + if (it->second) flattenNestedLiterals(*it->second); + } + if (!it->second) return nullptr; + return chunkEligibleNow(*it->second) ? &*it->second : nullptr; +} + void Evaluator::flattenNestedLiterals(CompiledChunk& chunk) { for (auto& [litNode, litChunk] : chunk.nestedLiterals) { flattenNestedLiterals(litChunk); @@ -38,6 +48,40 @@ void Evaluator::flattenNestedLiterals(CompiledChunk& chunk) { chunk.nestedLiterals.clear(); } +Value Evaluator::evalExprMaybeCompiled(const oscad::Expression& node, EvalContext& ctx) { + if (!useBytecodeVm() || !inResolvePass_) return evalExpr(node, ctx); + auto it = stmtExprChunkCache_.find(&node); + if (it == stmtExprChunkCache_.end()) { + it = stmtExprChunkCache_.emplace(&node, tryCompileStatementExpr(node, node.scope())).first; + // A zero-capture closure literal (e.g. `x = function(y) y + 1;`) + // still reaches chunk.nestedLiterals even though it never touches + // closureSites (see tryCompileStatementExpr's own doc comment: only + // a CAPTURES-HAVING closure is refused) -- flatten it into + // literalChunkCache_ exactly like lookupOrCompileChunk does, so a + // later call to that closure value can run compiled too. + if (it->second) flattenNestedLiterals(*it->second); + } + if (!it->second || !chunkEligibleNow(*it->second)) return evalExpr(node, ctx); + return runCompiledExprChunk(*this, *it->second, ctx); +} + +bool Evaluator::tryRunCompiledAssignmentBlock(const std::vector& assignments, + EvalContext& ctx) { + if (assignments.empty() || !useBytecodeVm() || !inResolvePass_) return false; + auto* first = static_cast(assignments.front()); + auto it = assignBlockChunkCache_.find(first); + if (it == assignBlockChunkCache_.end()) { + std::vector assigns; + assigns.reserve(assignments.size()); + for (const oscad::ASTNode* n : assignments) assigns.push_back(static_cast(n)); + it = assignBlockChunkCache_.emplace(first, tryCompileAssignmentBlock(assigns, first->scope())).first; + if (it->second) flattenNestedLiterals(*it->second); + } + if (!it->second || !chunkEligibleNow(*it->second)) return false; + runCompiledAssignmentBlock(*this, *it->second, ctx); + return true; +} + const Value* Evaluator::findUpvalue(const oscad::ASTNode* targetDecl, int slot) const { // Walks upvalueParent links, NOT a blind scan of callStack_ -- see // that field's own doc comment for the real bug this distinction @@ -98,11 +142,11 @@ BoundArgs Evaluator::bindArgs(const std::vectorkind() == oscad::NodeKind::NamedArgument) { auto& a = static_cast(*argPtr); - result.set(a.name->name, evalExpr(*a.expr, ctx)); + result.set(a.name->name, evalExprMaybeCompiled(*a.expr, ctx)); } else { auto& a = static_cast(*argPtr); if (positionalIdx < nparams) { - result.set(params[positionalIdx]->name->name, evalExpr(*a.expr, ctx)); + result.set(params[positionalIdx]->name->name, evalExprMaybeCompiled(*a.expr, ctx)); } ++positionalIdx; } @@ -136,10 +180,22 @@ EvalContext Evaluator::callCtxFor(const oscad::ASTNode& decl, EvalContext& ctx, if (usedChildCtx) *usedChildCtx = false; return ctx.callCtxFromCapturedLet(capturedLet, scope, std::nullopt, std::move(childrenNodes), childrenCallerCtx); } + // Was a full callStack_ scan (every frame, checking THAT frame's own + // declPosition for span containment) -- see activeDeclRefcount_'s own + // doc comment (evaluator.hpp) for why iterating its DISTINCT keys + // instead is exactly equivalent, not an approximation: containment for + // a given declaration depends only on that declaration's own + // (compile-time-fixed) position, never on which of its possibly-many + // active occurrences on callStack_ is checked, so deduplicating by + // declaration identity changes nothing observable -- only how many + // checks a deeply/self-recursive call chain (the overwhelmingly common + // shape once Stage 1/2 made deep recursion possible at all) needs to + // do: a small, roughly-constant number of DISTINCT active + // declarations, not one check per active call. const oscad::Position& declPos = decl.position(); - for (const CallStackFrame& frame : callStack_) { - const oscad::Position* outer = frame.declPosition; - if (outer == nullptr || outer->origin != declPos.origin) continue; + for (const auto& [activeDecl, count] : activeDeclRefcount_) { + const oscad::Position* outer = &activeDecl->position(); + if (outer->origin != declPos.origin) continue; const bool sameSpan = (outer->start_offset == declPos.start_offset && outer->end_offset == declPos.end_offset); if (!sameSpan && outer->start_offset <= declPos.start_offset && declPos.end_offset <= outer->end_offset) { if (usedChildCtx) *usedChildCtx = true; @@ -223,9 +279,9 @@ void Evaluator::bindCallArgsInto(const std::vector(); childrenNodes->reserve(call.children.size()); @@ -254,25 +310,79 @@ void Evaluator::evalUserModule(const oscad::ModuleDeclaration& decl, const oscad } applyDefaults(decl.parameters, bound, childCtx); - int parentModules = 0; - for (const CallStackFrame& f : callStack_) { - if (f.kind == CallStackFrame::Kind::Module) ++parentModules; - } - childCtx.dyn->set("$parent_modules", Value{static_cast(parentModules)}); + // moduleCallDepth_ (maintained incrementally by enterUserCall/ + // exitUserCall*) is already exactly "how many Module frames are on + // callStack_ right now" -- this call's OWN frame isn't pushed yet, so + // it correctly counts only ancestors, matching what a callStack_ scan + // would have found. See moduleCallDepth_'s own doc comment for why a + // scan here specifically used to be an O(depth) hazard. + childCtx.dyn->set("$parent_modules", Value{static_cast(moduleCallDepth_)}); + return childCtx; +} - std::optional prof = profileEnter("module", call.name->name, &call.position(), &decl.position()); - callStack_.push_back( - CallStackFrame{CallStackFrame::Kind::Module, call.name->name, &call.position(), &decl.position()}); - callStack_.back().bodyCtx = &childCtx; // per-frame locals for the debugger +void Evaluator::runModuleBodyNative(const oscad::ModuleDeclaration& decl, EvalContext& childCtx, + const oscad::Position* callPos) { + // Guards against a genuinely (non-tail) recursive user module the same + // way evalUserFunctionCore's own guard does for functions (evaluator.hpp) + // -- each logical module call HERE is a real, unavoidable C++ recursive + // call (evalModularCall -> evalUserModule -> evalChildren -> + // evalStatement -> evalModularCall -> ...), so a deep enough one + // silently overflows the native stack with no exception to catch. + // Confirmed present for a plain, closure-free `module recur(n) { if + // (n>0) recur(n-1); else cube(1); }` -- segfaults around depth 5000 on + // an ordinary desktop build. Recursive tree/pattern-generator modules + // are a real, common pattern (not just an adversarial script), so this + // is reachable in practice. Shares kMaxUserCallDepth AND callStack_ + // itself with the function-side guard: a function recursing into a + // module recursing into a function is the same hazard regardless of + // which kind of frame is on top. This guard -- unlike the compiled + // path's own (see pushBracketedModuleFrame, bytecode_vm.cpp, + // skipDepthGuard) -- stays exactly as-is: this IS still a genuine + // native recursive call, so native stack margin still matters here. + // Uses enterUserCall/exitUserCall* (not a hand-rolled profileEnter/ + // callStack_.push_back) so this bracket is identical in shape to the + // compiled path's own (evalUserModule, below) -- one shared mechanism, + // not two that could drift. + UserCallHandle h = enterUserCall(decl.name->name, decl, /*bodyExpr=*/nullptr, childCtx, callPos, + /*upvalueParent=*/-1, /*skipDepthGuard=*/false, CallStackFrame::Kind::Module); try { evalChildren(decl.children, childCtx); } catch (...) { - callStack_.pop_back(); - if (prof) profileExit(*prof); + exitUserCallException(h); throw; } - callStack_.pop_back(); - if (prof) profileExit(*prof); + exitUserCallSuccess(decl.name->name, h, Value{}, /*fireReturnHook=*/false); +} + +void Evaluator::evalUserModule(const oscad::ModuleDeclaration& decl, const oscad::ModularCall& call, EvalContext& ctx) { + auto bound = bindArgs(decl.parameters, call.arguments, ctx); + EvalContext childCtx = buildModuleChildCtx(decl, call, ctx, std::move(bound)); + const CompiledChunk* chunk = useBytecodeVm() ? lookupOrCompileModuleChunk(decl) : nullptr; + if (!chunk) { + runModuleBodyNative(decl, childCtx, &call.position()); + return; + } + // Unlike runModuleBodyNative, this bracket wraps runCompiledModuleBody + // instead of evalChildren -- but it's still REQUIRED here, even though + // runCompiledModuleBody's own frame is bare (see its own doc comment): + // this call site IS the "outer caller" that bare frame assumes already + // did the bracketing (mirrors evalUserFunctionCore's own unconditional + // enterUserCall wrapping BOTH its compiled and native branches + // uniformly). Skipping it here was a real bug caught by + // UserModule.NestedModuleSeesReassignedParameterViaClosure hanging -- + // without `decl`'s own CallStackFrame on callStack_, callCtxFor's own + // span-containment closure search (used by a module declared INSIDE + // this one's body) can never find it, silently misclassifying every + // such lookup. + UserCallHandle h = enterUserCall(decl.name->name, decl, /*bodyExpr=*/nullptr, childCtx, &call.position(), + /*upvalueParent=*/-1, /*skipDepthGuard=*/false, CallStackFrame::Kind::Module); + try { + runCompiledModuleBody(*this, *chunk, childCtx); + } catch (...) { + exitUserCallException(h); + throw; + } + exitUserCallSuccess(decl.name->name, h, Value{}, /*fireReturnHook=*/false); } // -- Tail-call optimization (interpreter path) ----------------------------- @@ -505,6 +615,13 @@ void Evaluator::recordTailCallHop(const std::string& calleeName, const oscad::AS error("Recursion detected calling function '" + calleeName + "'", calleeDecl); } CallStackFrame& frame = callStack_.back(); + // A tail hop swaps which declaration occupies THIS stack slot without + // a matching enter/exit pair of its own -- activeDeclRefcount_ (see + // its own doc comment, evaluator.hpp) has to be updated symmetrically + // here too, or it would count the OLD declaration as active forever + // and never see the NEW one at all. + noteActiveDeclExit(frame.declNode); + ++activeDeclRefcount_[&calleeDecl]; frame.name = calleeName; frame.declNode = &calleeDecl; frame.declPosition = &calleeDecl.position(); @@ -513,6 +630,54 @@ void Evaluator::recordTailCallHop(const std::string& calleeName, const oscad::AS profileRecordTailHop("function", calleeName, callPos, &calleeDecl.position()); } +// See these three methods' own doc comment (evaluator.hpp) for the split +// rationale. Body is a direct, behavior-preserving lift of what +// evalUserFunctionCore used to inline -- see this file's own git history +// for the pre-split version if a byte-for-byte diff is ever needed. +Evaluator::UserCallHandle Evaluator::enterUserCall(const std::string& name, const oscad::ASTNode& declNode, + const oscad::Expression* bodyExpr, EvalContext& childCtx, + const oscad::Position* callPos, int upvalueParent, + bool skipDepthGuard, CallStackFrame::Kind kind) { + const bool isModule = kind == CallStackFrame::Kind::Module; + if (!skipDepthGuard && callStack_.size() >= kMaxUserCallDepth) { + error("Recursion too deep while calling " + std::string(isModule ? "module" : "function") + " '" + name + "'", + declNode); + } + UserCallHandle h; + h.kind = kind; + h.declNode = &declNode; + h.prof = profileEnter(isModule ? "module" : "function", name, callPos, &declNode.position()); + callStack_.push_back(CallStackFrame{kind, name, callPos, &declNode.position(), &declNode, nullptr, upvalueParent}); + callStack_.back().bodyCtx = &childCtx; // per-frame locals for the debugger + if (isModule) ++moduleCallDepth_; + ++activeDeclRefcount_[&declNode]; + if (bodyExpr) checkDebug(*bodyExpr, childCtx); + lastCtx_ = &childCtx; + return h; +} + +void Evaluator::noteActiveDeclExit(const oscad::ASTNode* declNode) { + auto it = activeDeclRefcount_.find(declNode); + if (it == activeDeclRefcount_.end()) return; // defensive; should always be found + if (--it->second <= 0) activeDeclRefcount_.erase(it); +} + +void Evaluator::exitUserCallSuccess(const std::string& name, const UserCallHandle& handle, const Value& result, + bool fireReturnHook) { + if (fireReturnHook && debugHooks_.returnHook) debugHooks_.returnHook(name, result, static_cast(callStack_.size())); + callStack_.pop_back(); + if (handle.kind == CallStackFrame::Kind::Module) --moduleCallDepth_; + noteActiveDeclExit(handle.declNode); + if (handle.prof) profileExit(*handle.prof); +} + +void Evaluator::exitUserCallException(const UserCallHandle& handle) { + callStack_.pop_back(); + if (handle.kind == CallStackFrame::Kind::Module) --moduleCallDepth_; + noteActiveDeclExit(handle.declNode); + if (handle.prof) profileExit(*handle.prof); +} + Value Evaluator::evalUserFunction(const std::string& name, const oscad::FunctionDeclaration& decl, const std::vector>& arguments, EvalContext& ctx, const oscad::ASTNode* callNode) { @@ -534,7 +699,7 @@ Value Evaluator::evalUserFunction(const std::string& name, const oscad::Function const oscad::Position* callPos = callNode ? &callNode->position() : nullptr; return evalUserFunctionCore(name, decl, *decl.expr, childCtx, callPos, upvalueParent, [&]() -> Value { - return chunk ? runCompiledFunctionTrampoline(*this, *chunk, arguments, ctx, childCtx) + return chunk ? runCompiledFunction(*this, *chunk, arguments, ctx, childCtx) : evalFunctionBodyTrampoline(*decl.expr, childCtx); }); } @@ -553,7 +718,7 @@ Value Evaluator::evalUserFunctionFromBound(const std::string& name, const oscad: [&]() -> Value { return evalFunctionBodyTrampoline(*decl.expr, childCtx); }); } return evalUserFunctionCore(name, decl, *decl.expr, childCtx, callPos, upvalueParent, [&]() -> Value { - return runCompiledFunctionFromBoundTrampoline(*this, *chunk, bound, childCtx); + return runCompiledFunctionFromBound(*this, *chunk, bound, childCtx); }); } @@ -589,7 +754,7 @@ Value Evaluator::evalFunctionLiteral(const Closure& closure, // further given no test depends on it. return evalUserFunctionCore("", funcNode, *funcNode.body, childCtx, callPos, upvalueParent, [&]() -> Value { - return chunk ? runCompiledFunctionTrampoline(*this, *chunk, arguments, ctx, childCtx) + return chunk ? runCompiledFunction(*this, *chunk, arguments, ctx, childCtx) : evalFunctionBodyTrampoline(*funcNode.body, childCtx); }); } @@ -611,7 +776,7 @@ Value Evaluator::evalFunctionLiteralFromBound(const Closure& closure, BoundArgs [&]() -> Value { return evalFunctionBodyTrampoline(*funcNode.body, childCtx); }); } return evalUserFunctionCore("", funcNode, *funcNode.body, childCtx, callPos, upvalueParent, - [&]() -> Value { return runCompiledFunctionFromBoundTrampoline(*this, *chunk, bound, childCtx); }); + [&]() -> Value { return runCompiledFunctionFromBound(*this, *chunk, bound, childCtx); }); } Value Evaluator::evalFunctionCall(const oscad::PrimaryCall& node, EvalContext& ctx) { diff --git a/tests/test_bytecode_compiler.cpp b/tests/test_bytecode_compiler.cpp index 618802b..c275b23 100644 --- a/tests/test_bytecode_compiler.cpp +++ b/tests/test_bytecode_compiler.cpp @@ -4,6 +4,7 @@ #include "test_helpers.hpp" +#include #include #include #include @@ -536,17 +537,24 @@ TEST(BytecodeCompiler, MakeClosureLetsContainerCompileDespiteNonEscapingClosure) // fast-continue "on", nothing set) to actually observe whether `make` // runs compiled here. // - // 3 stops, not 4: `make`'s own body-entry (1), `g`'s own body-entry - // (1) -- `g` itself now also runs compiled (its captures-having chunk - // is registered too, not discarded -- see the FunctionLiteral case's - // own doc comment, bytecode_compiler.cpp), so it costs exactly one - // stop instead of the extra sub-expression checkpoint an interpreted - // call used to add on top -- plus the top-level echo() statement's own - // stop (module-level code is never compiled). + // 2 stops: `make`'s own body-entry (1), `g`'s own body-entry (1) -- + // `g` itself now also runs compiled (its captures-having chunk is + // registered too, not discarded -- see the FunctionLiteral case's own + // doc comment, bytecode_compiler.cpp), so it costs exactly one stop + // instead of the extra sub-expression checkpoint an interpreted call + // used to add on top. NOT a 3rd stop for the top-level echo() + // statement's own call-site anymore either (Phase 1, module/top-level + // compilation): `echo(make(10))`'s own argument now compiles too (see + // evalExprMaybeCompiled, evaluator.hpp), so make(10)'s call-site + // checkpoint (previously fired by evalFunctionCall's own checkDebug, + // evaluated as a plain interpreted expression) no longer fires -- + // exactly the same "compiled code has no sub-expression checkpoints" + // trade-off a compiled FUNCTION body's own internal calls already had, + // now also applying to a statement's own top-level expression. const int stops = countDebugHookStops("function make(x) = let(g = function(y) y + x) g(5);\n" "echo(make(10));", std::unordered_map>{}); - EXPECT_EQ(stops, 3); + EXPECT_EQ(stops, 2); } TEST(BytecodeCompiler, MakeClosureLetsContainerCompileDespiteEscapingClosure) { @@ -563,12 +571,24 @@ TEST(BytecodeCompiler, MakeClosureLetsContainerCompileDespiteEscapingClosure) { // a checkpoint), so it produces the SAME stop count whether `outer` // compiled or not -- confirmed empirically, not assumed, by diffing // this test's own result against the pre-Op::MakeClosure compiler. + // + // 3, not 6: BOTH top-level statements' own expressions now compile too + // (Phase 1, module/top-level compilation). `stored = outer(5);` is + // ALSO its own compiled assignment BLOCK now (evalChildren's own + // leading assignment-run, a single-assignment block here -- see + // tryRunCompiledAssignmentBlock, evaluator.hpp), which bypasses + // evalChildren's own per-STATEMENT checkDebug entirely on top of + // `outer(5)`'s own call-site checkpoint already being lost (same + // reasoning as evalExprMaybeCompiled's doc comment) -- 2 stops lost + // there, not 1. `echo(stored(3));` (doEcho, not a batched assignment) + // still only loses its own call-site checkpoint the ordinary way -- 1 + // stop. 6 - 2 - 1 = 3. const int stops = countDebugHookStops( "function outer(n) = n > 0 ? function(y) y + n : function(y) y - n;\n" "stored = outer(5);\n" "echo(stored(3));", std::unordered_map>{}); - EXPECT_EQ(stops, 6); + EXPECT_EQ(stops, 3); } TEST(BytecodeCompiler, MakeClosureCapturesFreshValuePerInvocation) { @@ -653,18 +673,30 @@ TEST(BytecodeCompiler, SelfReferentialRecursiveClosureNowResolvesAsUpvalueAndRun // 32,000-element list (7+ seconds -> 0.05s once `a` genuinely // resolves itself as an upvalue instead). // - // 11 stops, not 30 (confirmed by diffing against the previous, - // containsLoadFree-excluded behavior with the same script): `reduce`'s - // own body-entry (1) + `a`'s own 5 recursive calls (1 each, compiled) - // + `func`'s own 4 calls (1 each, already compiled even before this -- - // zero captures) + the top-level echo() statement's own stop (module - // code is never compiled). + // 6 stops (was 11 before Phase 1 module/top-level compilation -- + // confirmed via a line/depth debug-hook dump, not re-derived by hand): + // the top-level echo() statement's own stop (1, line 2, depth 0) + + // `reduce`'s own body-entry (1, line 1, depth 1) + `func`'s own body- + // entry, once per call from within `a`'s 5-iteration chain (4, line 2, + // depth 2 -- `function(p,q) p+q` is textually written on line 2, as + // part of echo's own argument expression). `a`'s own 5 self-recursive + // hops contribute NOTHING of their own: reduce's own tail-position call + // INTO `a` (`a(init, 0)`, reduce's whole body) and every one of `a`'s + // own tail-recursive hops are all part of the SAME trampolined chain, + // so only the very first evalUserFunctionCore invocation (reduce's own) + // ever fires a body-entry checkDebug -- exactly the same collapse + // MutualRecursionBetweenSiblingLetBoundClosuresResolvesCorrectly, below, + // documents for isEven<->isOdd. What actually changed here: echo's own + // argument -- the WHOLE reduce(...) call -- now compiles as a bare + // statement-context chunk too (Phase 1, see evalExprMaybeCompiled, + // evaluator.hpp), so it no longer gets a separate call-site checkpoint + // of its own either, on top of everything already collapsed above. const int stops = countDebugHookStops( "function reduce(func, list, init=0) = let(l = len(list), a = function (x,i) i>{}); - EXPECT_EQ(stops, 11); + EXPECT_EQ(stops, 6); EXPECT_EQ(runCapturingEcho("function reduce(func, list, init=0) = let(l = len(list), a = function (x,i) " "iisOdd(5)->isEven(4)->...->isEven(0)): // isEven<->isOdd calling each other in tail position is exactly what // the tail-call trampoline collapses into ONE evalUserFunctionCore // invocation for the WHOLE chain (see runCompiledFunctionTrampoline, // bytecode_vm.cpp) -- its own unconditional body-entry checkDebug() // fires once for that entire loop, not once per hop. isEvenTest's own - // entry (1) + that one trampoline loop (1) + the top-level echo() - // statement's own stop (module code is never compiled) = 3. + // entry (1) + that one trampoline loop (1) = 2. NOT a 3rd stop for the + // top-level echo() statement's own call-site anymore either (Phase 1, + // module/top-level compilation): `echo(isEvenTest(6))`'s own argument + // now compiles too (see evalExprMaybeCompiled, evaluator.hpp), so + // isEvenTest(6)'s call-site checkpoint -- previously fired by plain + // interpreted evalFunctionCall -- no longer does. const int stops = countDebugHookStops( "function isEvenTest(n) = let(isEven = function(k) k==0 ? true : isOdd(k-1), isOdd = function(k) " "k==0 ? false : isEven(k-1)) isEven(n);\n" "echo(isEvenTest(6));", std::unordered_map>{}); - EXPECT_EQ(stops, 3); + EXPECT_EQ(stops, 2); EXPECT_EQ(runCapturingEcho("function isEvenTest(n) = let(isEven = function(k) k==0 ? true : isOdd(k-1), " "isOdd = function(k) k==0 ? false : isEven(k-1)) isEven(n);\n" "echo(isEvenTest(6));\necho(isEvenTest(7));"), @@ -772,11 +808,17 @@ TEST(BytecodeCompiler, ClosureWithDollarParameterNowCompilesToo) { // 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. + // DebugAttachedWithoutFastContinueAlwaysInterprets above). The + // breakpoint sits on line 2 -- echo's own statement line -- so it + // deliberately forces THAT one call (echo's `outer(10)` argument) to + // interpret, while outer's and g's own bodies (line 1) stay eligible + // and run compiled. 3 stops: the echo statement's own per-statement + // checkpoint (line 2), the interpreted call-site checkpoint for + // calling outer (line 2), and outer's own body-entry (line 1) -- g's + // own tail call, reached from INSIDE outer's now-compiled body, hops + // in place silently (no separate stop), proving the tail hop stays + // silent even for a closure capturing an enclosing binding with its + // own $-parameter. const int stops = countDebugHookStops(script, std::unordered_map>{{"", {2}}}); EXPECT_EQ(stops, 3); } @@ -1124,11 +1166,20 @@ TEST(BytecodeCompiler, FastContinueWithBreakpointInsideFunctionStillInterprets) ScopedVm vm(true); // Breakpoint on line 1 itself -- inside f's own compiled span -- must // still force the interpreter for f specifically, even in fast-continue - // mode: something on that exact line could need a real checkpoint. + // mode: something on that exact line could need a real checkpoint. 4, + // not 5 (Phase 1, module/top-level compilation): the breakpoint is on + // line 1 (f's own body), not line 2, so it doesn't affect echo(f(5))'s + // OWN argument expression -- a SEPARATE chunk with its own SEPARATE + // [minLine,maxLine] span (line 2 only) -- which is still eligible to + // compile and so loses its own call-site checkpoint, exactly like + // every other statement-context expression now does when eligible. + // f's OWN body still correctly interprets either way (the actual + // guarantee this test exists to prove), just via one fewer surrounding + // stop than before. const int stops = countDebugHookStops("function f(x) = x > 0 ? x + 1 : x - 1;\n" "echo(f(5));", std::unordered_map>{{"", {1}}}); - EXPECT_EQ(stops, 5); + EXPECT_EQ(stops, 4); } TEST(BytecodeCompiler, FastContinueWithBreakpointInDifferentFunctionUsesVmForThisOne) { @@ -1140,11 +1191,16 @@ TEST(BytecodeCompiler, FastContinueWithBreakpointInDifferentFunctionUsesVmForThi // this same file. Confirms the gating is genuinely per-function, not // "any breakpoint anywhere disables the whole file." Only f is ever // called, so g's own eligibility is never directly observed here. + // 2, not 3 (Phase 1, module/top-level compilation): echo(f(5))'s own + // argument (line 3, outside the breakpoint's line-2 span) is a + // separate chunk from f's own body, independently eligible to compile + // -- losing its own call-site checkpoint like every other eligible + // statement-context expression now does. const int stops = countDebugHookStops("function f(x) = x > 0 ? x + 1 : x - 1;\n" "function g(x) = x > 0 ? x + 1 : x - 1;\n" "echo(f(5));", std::unordered_map>{{"", {2}}}); - EXPECT_EQ(stops, 3); + EXPECT_EQ(stops, 2); } TEST(BytecodeCompiler, CStyleForIncrNameCanReadItsOwnPriorValueInTheSameAssignment) { @@ -1176,3 +1232,198 @@ TEST(BytecodeCompiler, CStyleForIncrNameCanReadItsOwnPriorValueInTheSameAssignme "echo(myloop(5));"), "ECHO: [4]"); } + +// -- Module-body compilation (Stage 2) ------------------------------------- +// +// Mirrors the function-side Phase B section above: module bodies now +// compile too (Op::CallModule/NativeStatement/NativeCondJumpIfFalse/ +// NativeIterMaterialize/ForIterNext/ForIterEnd -- see bytecode.hpp and +// tryCompileModuleBody, bytecode_compiler.cpp), specifically so a +// recursive module call no longer makes an unbounded native C++ recursive +// call the way evalUserModule always used to (no trampoline exists for +// modules, unlike functions' own tail-call machinery). assignment/echo/ +// assert/let-blocks/modifiers/intersection_for still delegate to a native +// evalStatement call for one node at a time -- only if/for/a resolved +// user-module call get real bytecode, since those are the shapes that can +// hide a recursive call behind a native call boundary. + +TEST(ModuleBodyCompiles, ForLoopProducesCorrectGeometryCompiled) { + // Each iteration's translate()/cube() stays its own separate top-level + // body (four disjoint cubes, not boolean-unioned into one manifold -- + // confirmed against the interpreted path directly: byte-identical STL + // output either way for this exact script). Asserting bodies.size() + // and the outermost bounding box across all of them is what's actually + // load-bearing here, not a specific body count guess. + ScopedVm vm(true); + Evaluated e = evalSrc("module row(n) { for (i = [0:n-1]) translate([i * 2, 0, 0]) cube(1); }\nrow(4);"); + ASSERT_EQ(e.bodies.size(), 4u); + double maxX = 0.0; + for (const auto& body : e.bodies) maxX = std::max(maxX, body.body->BoundingBox().max.x); + EXPECT_NEAR(maxX, 7.0, 1e-9); // last cube at i=3 -> translate([6,0,0]), cube(1) -> max.x = 7 +} + +TEST(ModuleBodyCompiles, IfElseInModuleBodyPicksCorrectBranchCompiled) { + ScopedVm vm(true); + Evaluated eTrue = evalSrc("module pick(n) { if (n > 0) cube(5); else sphere(1); }\npick(1);"); + ASSERT_EQ(eTrue.bodies.size(), 1u); + EXPECT_NEAR(eTrue.bodies[0].body->BoundingBox().max.x, 5.0, 1e-9); + + Evaluated eFalse = evalSrc("module pick(n) { if (n > 0) cube(5); else sphere(1); }\npick(-1);"); + ASSERT_EQ(eFalse.bodies.size(), 1u); + EXPECT_NEAR(eFalse.bodies[0].body->BoundingBox().max.x, 1.0, 1e-9); +} + +TEST(ModuleBodyCompiles, ReassignmentInsideForLoopBodyDoesNotSpuriouslyWarn) { + ScopedVm vm(true); + // Op::ForIterNext pushes a FRESH child ctx per iteration (see its own + // doc comment, bytecode.hpp) specifically so an ordinary local + // assignment inside the loop body -- reusing the same NAME every + // iteration, a common and unremarkable pattern -- doesn't trip + // evalAssignment's own "was assigned on line N but overwritten" + // warning from iteration 2 onward. Reusing the SAME ctx across + // iterations (the wrong, simpler design) would warn 9 times here. + std::vector warnings; + Evaluator ev([&](const std::string& msg) { warnings.push_back(msg); }); + auto ast = parseSrc("module row(n) { for (i = [0:n-1]) { r = i * 2; translate([r, 0, 0]) cube(1); } }\nrow(10);"); + auto scope = oscad::buildScopes(ast); + EvalContext ctx = EvalContext::makeRoot(scope.get()); + ev.evaluate(ast, ctx, {}); + for (const std::string& w : warnings) EXPECT_EQ(w.find("overwritten"), std::string::npos) << w; +} + +TEST(ModuleBodyCompiles, RecursiveModuleWithIfSucceedsWellPastTheOldNativeLimitCompiled) { + ScopedVm vm(true); + Evaluated e = evalSrc("module recur(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\nrecur(10000);"); + ASSERT_EQ(e.bodies.size(), 1u); +} + +TEST(ModuleBodyCompiles, RecursiveModuleThrowingPartwayUnwindsCleanlyAndVmFrameRunsAgain) { + ScopedVm vm(true); + Evaluator ev; + auto ast1 = + parseSrc("module recur(n) { if (n > 0) { recur(n - 1); } else { assert(false, \"boom\"); } }\nrecur(5000);"); + auto scope1 = oscad::buildScopes(ast1); + EvalContext ctx1 = EvalContext::makeRoot(scope1.get()); + EXPECT_THROW(ev.evaluate(ast1, ctx1, {}), EvalError); + + // A second, unrelated script on the SAME Evaluator afterward -- if any + // VmFrame/treeStack_ frame/callStack_ entry leaked from the throw + // above, this would crash, misbehave, or show a corrupted pool. + auto ast2 = parseSrc("module recur(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\nrecur(2000);"); + auto scope2 = oscad::buildScopes(ast2); + EvalContext ctx2 = EvalContext::makeRoot(scope2.get()); + std::vector bodies = ev.evaluate(ast2, ctx2, {}); + ASSERT_EQ(bodies.size(), 1u); +} + +TEST(ModuleBodyCompiles, DollarVarSetEarlyStaysVisibleDeepInARecursiveModuleChainCompiled) { + ScopedVm vm(true); + // The module-call analog of TailCalls.DollarVarSetEarlyInANonTailCompiled + // ChainStaysVisibleManyLevelsDeep (test_tail_calls.cpp) -- each + // recursive level is a genuine Op::CallModule push (its own VmFrame, + // its own ctxChain entry via buildModuleChildCtx's callCtxFor), not a + // hop -- $probe (dyn, isolate=false) must still resolve 2000 real + // pushes deep. + EXPECT_EQ(runCapturingEcho("module recur(n) { if (n > 0) { recur(n - 1); } else { echo($probe); } }\n" + "let($probe = 77) recur(2000);"), + "ECHO: 77"); +} + +TEST(ModuleBodyCompiles, NestedModuleClosureOverReassignedParameterCompiles) { + ScopedVm vm(true); + // Regression test for a real bug caught while building this feature: + // evalUserModule's own compiled branch used to skip the callStack_ + // bracket entirely (assuming, wrongly, that some OUTER caller already + // did it -- true for a NESTED Op::CallModule call, never true for + // THIS, the outermost entry point) -- silently breaking callCtxFor's + // span-containment closure search for any module declared INSIDE + // another module's body, which hung (an infectious infinite loop, not + // a clean failure) rather than crashing outright. + Evaluated e = evalSrc("module outer(edges) {" + " edges = edges * 2;" + " module inner() { cube(edges); }" + " inner();" + "}" + "outer(3);"); + ASSERT_EQ(e.bodies.size(), 1u); + EXPECT_NEAR(e.bodies[0].body->BoundingBox().max.x, 6.0, 1e-9); +} + +TEST(ModuleBodyCompiles, ParentModulesCountIsAccurateAtEachNestingLevelCompiled) { + // Evaluator::moduleCallDepth_ replaced a full callStack_ rescan per + // module call (buildModuleChildCtx's own $parent_modules) with an + // incrementally maintained counter -- found necessary because the + // rescan is O(depth), turning a script recursing 64,000 modules deep + // (no geometry cost at all, just the recursion itself) from an + // expected-linear ~1s into an actual ~5.5s, and 128,000 deep into + // ~22s (quadratic, confirmed by doubling depth and watching wall time + // roughly quadruple) -- entirely invisible at the small depths any of + // this session's OTHER tests exercise. This test is the correctness + // half of that fix: increment/decrement must land on exactly the + // right call, at every depth, or $parent_modules silently drifts. + EXPECT_EQ(runCapturingEcho("module inner() { echo($parent_modules); }\n" + "module mid() { inner(); }\n" + "module outer() { mid(); }\n" + "outer();"), + "ECHO: 2"); +} + +TEST(ModuleBodyCompiles, ParentModulesCountRecoversCorrectlyAfterACaughtExceptionCompiled) { + // The exception-unwind path (Evaluator::exitUserCallException, and + // driveVm's own teardownVmCallStackDownTo for a compiled chain) must + // decrement moduleCallDepth_ exactly as often as the success path + // does -- otherwise a caught error inside a deep module chain would + // leave every SUBSEQUENT $parent_modules read permanently wrong (an + // off-by-N leak, not a crash, so nothing else would catch it). + std::string captured; + Evaluator ev([&](const std::string& msg) { captured = msg; }); + auto ast1 = parseSrc("module inner() { assert(false, \"boom\"); }\n" + "module outer() { inner(); }\n" + "outer();"); + auto scope1 = oscad::buildScopes(ast1); + EvalContext ctx1 = EvalContext::makeRoot(scope1.get()); + EXPECT_THROW(ev.resolveTree(ast1, ctx1), EvalError); + + // Same Evaluator instance, a second, unrelated script -- if the throw + // above leaked moduleCallDepth_ increments, THIS $parent_modules read + // would be wrong even though nothing here is nested at all. + auto ast2 = parseSrc("module inner() { echo($parent_modules); }\n" + "module outer() { inner(); }\n" + "outer();"); + auto scope2 = oscad::buildScopes(ast2); + EvalContext ctx2 = EvalContext::makeRoot(scope2.get()); + ev.resolveTree(ast2, ctx2); + EXPECT_EQ(captured, "ECHO: 1"); +} + +TEST(ModuleBodyCompiles, NestedModuleClosureStillWorksAfterADeepUnrelatedRecursiveDetour) { + ScopedVm vm(true); // 5000-deep non-tail recursion needs the compiled path (see test_tail_calls.cpp) + // Regression test for Evaluator::activeDeclRefcount_ (callCtxFor's own + // O(depth) callStack_ scan replaced with a refcounted set of DISTINCT + // active declarations -- see its own doc comment, evaluator.hpp, for + // the O(depth^2) perf cliff this fixes). The refcount for `deep` + // should go all the way back to exactly 0 once its 5000-deep + // recursive detour unwinds -- if the enter/exit accounting were even + // slightly off (e.g. missing a decrement somewhere), `deep`'s own + // entry would leak into activeDeclRefcount_ forever, which wouldn't + // break correctness here (deep's own span can't contain outer/inner's) + // but WOULD mean this test is exercising a real leak undetected by + // simpler tests. Bounding `deep`'s own recursion at a shallow depth + // first (to confirm it terminates cleanly) then going deep is the + // point: it must not corrupt the SEPARATE outer/inner closure check + // that runs after it, on the same Evaluator. + std::string captured; + Evaluator ev([&](const std::string& msg) { captured = msg; }); + auto ast = parseSrc("function deep(n) = n <= 0 ? 0 : 1 + deep(n - 1);\n" + "module outer(edges) {" + " edges = edges * 2;" + " module inner() { echo(edges); }" + " inner();" + "}" + "echo(deep(5000));" + "outer(3);"); + auto scope = oscad::buildScopes(ast); + EvalContext ctx = EvalContext::makeRoot(scope.get()); + ev.resolveTree(ast, ctx); + EXPECT_EQ(captured, "ECHO: 6"); // 3*2, seen via inner's own closure over outer's reassigned edges +} diff --git a/tests/test_control_flow.cpp b/tests/test_control_flow.cpp index 6b7bed0..a340259 100644 --- a/tests/test_control_flow.cpp +++ b/tests/test_control_flow.cpp @@ -10,6 +10,16 @@ using namespace oscadeval::test; namespace { +// Mirrors test_bytecode_compiler.cpp's/test_tail_calls.cpp's own ScopedVm -- +// the plain OSCAD_BYTECODE_VM env var is cached forever after its first +// read, so forcing it per-test needs this override instead (see +// Evaluator::setBytecodeVmEnabledForTesting's own doc comment). +class ScopedVm { +public: + explicit ScopedVm(bool enabled) { Evaluator::setBytecodeVmEnabledForTesting(enabled); } + ~ScopedVm() { Evaluator::setBytecodeVmEnabledForTesting(std::nullopt); } +}; + double asNum(const Value& v) { return std::get(v); } // Runs `code` as a full top-level script (all statements, not just a bare @@ -670,6 +680,28 @@ TEST(UserModule, RecursiveModuleCall) { EXPECT_EQ(e.bodies.size(), 3u); } +TEST(UserModule, DeepNonTailRecursionHitsAControlledErrorInterpreted) { + // Interpreted modules still have no trampoline and no compiled path + // (see Op::CallModule/tryCompileModuleBody, Stage 2) -- every logical + // recursive call here is still a real, unavoidable C++ recursive call, + // so this guard still matters for this path specifically. The + // COMPILED path's own version of this exact script now SUCCEEDS + // instead -- see BytecodeCompiler. + // RecursiveModuleWithIfSucceedsWellPastTheOldNativeLimitCompiled + // (test_bytecode_compiler.cpp) -- these two together are the + // differential proof Stage 2 actually changed something, not a + // reversion. + ScopedVm vm(false); + try { + evalSrc("module recur(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\nrecur(500);"); + FAIL() << "expected EvalError"; + } catch (const EvalError& e) { + const std::string what = e.what(); + EXPECT_NE(what.find("Recursion too deep"), std::string::npos); + EXPECT_NE(what.find("'recur'"), std::string::npos); + } +} + TEST(UserModule, DefaultParameterApplied) { Evaluated e = evalSrc("module box(size=5) { cube(size); }\nbox();"); manifold::Box bbox = e.bodies[0].body->BoundingBox(); diff --git a/tests/test_debug_hooks.cpp b/tests/test_debug_hooks.cpp index d20503e..e814dfd 100644 --- a/tests/test_debug_hooks.cpp +++ b/tests/test_debug_hooks.cpp @@ -302,7 +302,19 @@ TEST(DebugHooks, FastContinueNotHookSkippableStillFiresEveryCheckpoint) { auto scope = oscad::buildScopes(ast); EvalContext ctx = EvalContext::makeRoot(scope.get()); ev.evaluate(ast, ctx); - EXPECT_EQ(calls, 7); // unaffected -- same total as HookFiresOnceForEachStatementAtEveryNestingLevel + // 4, not 7 (Phase 1, module/top-level compilation): hookSkippable is a + // narrower claim than "breakpoints is accurate" (see + // setFastContinueBreakpoints' own doc comment, evaluator.hpp) -- it + // does NOT disable useBytecodeVm()/chunkEligibleNow, only whether + // checkDebug() can skip calling the hook at all for an eligible line. + // With an empty (but real) breakpoint set, translate()'s own argument + // `[1,0,0]` is now eligible to compile too (evalExprMaybeCompiled, + // called from resolveArgs) -- as a single Op::BuildList instead of + // three separate _eval_list_comp element evaluations, its own 3 + // expr-level element checkpoints (see + // HookFiresOnceForEachStatementAtEveryNestingLevel's own doc comment, + // above, for where those 3 came from) no longer fire: 7 - 3 = 4. + EXPECT_EQ(calls, 4); } TEST(DebugHooks, FastContinueInterruptFlagForcesTheNextCheckpointThrough) { diff --git a/tests/test_tail_calls.cpp b/tests/test_tail_calls.cpp index a2a7576..99f5cd4 100644 --- a/tests/test_tail_calls.cpp +++ b/tests/test_tail_calls.cpp @@ -316,19 +316,72 @@ TEST(TailCalls, DeepNonTailRecursionHitsAControlledErrorInsteadOfCrashingInterpr } } -TEST(TailCalls, DeepNonTailRecursionHitsAControlledErrorInsteadOfCrashingCompiled) { +TEST(TailCalls, DeepNonTailRecursionUnderVmDoesNotHitTheNativeDepthGuard) { ScopedVm vm(true); - // Same script, same guard (evalUserFunctionCore is shared by both the - // interpreter and the bytecode VM's own compiled call path) -- the - // compiled path's own real per-call native stack usage is actually - // larger (more C++ frames between one logical call and the next, see - // runChunk/runCompiledFunctionFromBound/runCompiledFunctionFromBoundTrampoline), - // so this is the MORE exposed of the two paths, not a lesser check. + // Same script as the interpreted test above, but this is exactly the + // case the explicit frame-stack VM (see bytecode_vm.cpp's driveVm) + // exists to fix: a NON-tail call between two compiled chunks + // (Op::CallFn, via pushBracketedCallFrame) no longer makes a genuine + // native C++ call -- it's pushed onto vmCallStack_ and serviced by + // driveVm's own loop instead, so kMaxUserCallDepth (50, a NATIVE + // stack margin -- see its own doc comment) no longer applies here at + // all (enterUserCall's skipDepthGuard=true, set only by + // pushBracketedCallFrame). f(500) now succeeds where it used to throw + // -- the direct, measurable proof the redesign works, not just "no + // regression." Bounded only by the new, much larger + // kMaxVmCallStackDepth (1,000,000, a memory/runaway guard, not a + // stack-safety one). Evaluator ev; - auto ast = parseSrc("function f(n) = n <= 0 ? 0 : 1 + f(n - 1);\nresult = f(500);"); + auto ast = parseSrc("function f(n) = n <= 0 ? 0 : 1 + f(n - 1);\necho(f(500));"); + auto scope = oscad::buildScopes(ast); + EvalContext ctx = EvalContext::makeRoot(scope.get()); + ev.resolveTree(ast, ctx); +} + +TEST(TailCalls, DeepNonTailRecursionUnderVmSucceedsWellPastTheOldNativeLimit) { + ScopedVm vm(true); + // 10,000 is two orders of magnitude past the old native-stack-bound + // kMaxUserCallDepth=50 -- the explicit-stack VM (vmCallStack_, heap- + // allocated) has no trouble with it. + const std::string script = "function f(n) = n <= 0 ? 0 : 1 + f(n - 1);\necho(f(10000));"; + EXPECT_EQ(runCapturingEcho(script), "ECHO: 10000"); +} + +TEST(TailCalls, DeepNonTailRecursionThrowingPartwayUnwindsCleanly) { + ScopedVm vm(true); + // An assert() partway down a long NON-tail compiled call chain throws + // through N pushed, bracketed VmFrames -- driveVm's own outer + // try/catch (teardownVmCallStackDownTo) must walk vmCallStack_ back + // to the floor, releasing every VmFrame to the pool and closing out + // every callStack_/profiling bracket WITHOUT firing returnHook, + // exactly mirroring the pre-redesign per-frame native unwind. Proven + // not by inspecting internals directly but by running a second, + // unrelated script on the SAME Evaluator afterward -- if any frame, + // pool slot, or callStack_ entry leaked, this second run would either + // crash, wrongly inherit stale state, or the pool would be visibly + // exhausted/corrupted. + Evaluator ev; + auto ast = parseSrc("function f(n) = n <= 0 ? assert(false, \"boom\") : 1 + f(n - 1);\necho(f(5000));"); auto scope = oscad::buildScopes(ast); EvalContext ctx = EvalContext::makeRoot(scope.get()); EXPECT_THROW(ev.resolveTree(ast, ctx), EvalError); + + auto ast2 = parseSrc("function g(n) = n <= 0 ? 0 : 1 + g(n - 1);\necho(g(2000));"); + auto scope2 = oscad::buildScopes(ast2); + EvalContext ctx2 = EvalContext::makeRoot(scope2.get()); + ev.resolveTree(ast2, ctx2); +} + +TEST(TailCalls, DollarVarSetEarlyInANonTailCompiledChainStaysVisibleManyLevelsDeep) { + ScopedVm vm(true); + // The ctxChain analog of DollarVarSetEarlyInATailChainStaysVisible + // ManyHopsLater above, but for a NON-tail chain -- each level is a + // genuine pushBracketedCallFrame push (its own VmFrame, its own + // ctxChain entry), not a tail hop mutating one frame in place. $probe + // (dyn, isolate=false) must still resolve 2000 real pushes deep. + const std::string script = "function depth(n) = n <= 0 ? $probe : 1 + depth(n - 1);\n" + "echo(let($probe = 77) depth(2000));"; + EXPECT_EQ(runCapturingEcho(script), "ECHO: 2077"); } TEST(TailCalls, CollatzStepsRecursesFromBothTernaryBranchesInterpreted) { From 4478a57e52b651a5df5d864b75a30fa50907fcbb Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 03:39:29 -0700 Subject: [PATCH 2/5] Force VM-on in a test whose stop-count depends on it FastContinueNotHookSkippableStillFiresEveryCheckpoint's own expected count (4, not 7) depends on the compiled path actually running (translate()'s own [1,0,0] argument becomes compile-eligible, collapsing 3 sub-expression stops) -- it never forced VM state, just relied on the process default, so it silently broke whenever CI (or anyone locally) ran the suite with OSCAD_BYTECODE_VM=0. Caught by PR #59's own CI (macOS/Ubuntu legs both failed on exactly this). Co-Authored-By: Claude Sonnet 5 --- tests/test_debug_hooks.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_debug_hooks.cpp b/tests/test_debug_hooks.cpp index e814dfd..3998424 100644 --- a/tests/test_debug_hooks.cpp +++ b/tests/test_debug_hooks.cpp @@ -17,6 +17,20 @@ using namespace oscadeval; using namespace oscadeval::test; +namespace { +// Mirrors test_bytecode_compiler.cpp's/test_tail_calls.cpp's own ScopedVm -- +// the plain OSCAD_BYTECODE_VM env var is cached forever after its first +// read, so a test whose own expected stop-count depends on the compiled +// path actually running (see FastContinueNotHookSkippableStillFiresEvery +// Checkpoint, below) must force it explicitly rather than relying on +// whatever the process default happens to be. +class ScopedVm { +public: + explicit ScopedVm(bool enabled) { Evaluator::setBytecodeVmEnabledForTesting(enabled); } + ~ScopedVm() { Evaluator::setBytecodeVmEnabledForTesting(std::nullopt); } +}; +} // namespace + // Records (line, exprLevel, forced) for every debug-hook call, in order -- // the shape every parity test below asserts on. Sequences here were taken // from the Python reference (openscad_evaluator) run over the same source @@ -284,6 +298,14 @@ TEST(DebugHooks, FastContinueHookSkippableStillFiresAtMatchingBreakpointLine) { } TEST(DebugHooks, FastContinueNotHookSkippableStillFiresEveryCheckpoint) { + // This test's own expected count (4, not 7) depends on the compiled + // path actually running (translate()'s own [1,0,0] argument becomes + // compile-eligible, collapsing 3 sub-expression stops -- see below) -- + // force it rather than relying on the process-default OSCAD_BYTECODE_VM, + // or this test is silently wrong half the time CI runs the suite with + // it forced off (caught for real: both macOS/Ubuntu CI legs failed on + // exactly this, PR #59). + ScopedVm vm(true); int calls = 0; DebugHooks hooks; hooks.debugHook = [&](int, int, bool, bool, const std::string&, const std::vector&, const DebugFramesFn&) { From 0d9c600bdf608991ca898d6d218c95628da64a20 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 03:53:09 -0700 Subject: [PATCH 3/5] Lower kMaxUserCallDepth: Stage 2 added a sustained native frame Factoring evalUserModule's own native-fallback body into runModuleBodyNative (shared with Op::CallModule's own native-fallback branch, so neither duplicates the enterUserCall/evalChildren/ exitUserCall bracket) added one sustained extra native stack frame to every level of INTERPRETED module recursion -- the one recursive shape this cap still protects (compiled module recursion runs through the explicit vmCallStack_ instead and never touches this guard at all). 50 was tight enough on Windows CI specifically that this alone segfaulted UserModule.DeepNonTailRecursionHitsAControlledError Interpreted instead of throwing cleanly at the guard (PR #59, both macOS/Ubuntu CI legs already passed; Windows didn't). Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/evaluator.hpp | 24 ++++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 1777719..8d898ff 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -1334,11 +1334,25 @@ class Evaluator { // evalUserFunctionCore (below) and evalUserModule (user_calls.cpp) -- // see evalUserFunctionCore's own doc comment for the full calibration // story (two rounds of CI stack-depth diagnostics on the worst-case - // platform). One constant because both push onto the SAME callStack_ -- - // a function recursing into a module recursing into a function is the - // same native-stack-growth hazard regardless of which kind of frame is - // on top at any given depth. - static constexpr size_t kMaxUserCallDepth = 50; + // platform -- Windows, whose default thread stack is much smaller than + // macOS/Linux's). One constant because both push onto the SAME + // callStack_ -- a function recursing into a module recursing into a + // function is the same native-stack-growth hazard regardless of which + // kind of frame is on top at any given depth. + // + // Re-lowered from 50 (Stage 2, module-body compilation): factoring + // evalUserModule's own native-fallback body into a separate + // runModuleBodyNative function (shared with Op::CallModule's own + // native-fallback branch, bytecode_vm.cpp -- needed so neither + // duplicates the enterUserCall/evalChildren/exitUserCall bracket) + // added ONE sustained extra native stack frame to every level of + // INTERPRETED module recursion (the one recursive shape this cap still + // protects -- compiled module recursion runs through the explicit + // vmCallStack_ instead, see kMaxVmCallStackDepth, and never touches + // this at all). 50 was tight enough on Windows CI that this alone + // segfaulted UserModule.DeepNonTailRecursionHitsAControlledError + // Interpreted (PR #59) instead of throwing cleanly at the guard. + static constexpr size_t kMaxUserCallDepth = 30; // See lastChildrenPositions()'s own doc comment. std::optional>> lastChildrenPositions_; From 72100d2a69a3ccc0da0e7e84078c9b4f14be474b Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 09:51:53 -0700 Subject: [PATCH 4/5] Guard CSG tree depth at construction, not by walking it later Stage 2 made deep recursive module chains resolve successfully, but that only bounds how many CALLS happen -- not how deep the CSGNode TREE those calls build ends up being. A module-call chain that wraps a single spliced child stays tree-depth 1 no matter how deep it recurses (evalModularCall's own splice logic collapses it away), but an ordinary "chain/spiral of shapes" pattern -- adding its own incremental geometry each level, e.g. `cube(0.1); recur(n-1);` -- makes each level's own splice see >1 child, so it gets wrapped in a synthetic union CSGNode instead, and the tree genuinely grows one level per call. Before this, such a chain resolved fine (confirmed directly: a standalone probe showed resolveTree() itself returning successfully) and then segfaulted the instant the resulting tree went out of scope -- std::unique_ptr's own default recursive destructor is an un-guardable native-stack risk once a tree that deep exists at all. A destructor can't throw a catchable error and a real stack overflow isn't a C++ exception, so the only fix is to stop the tree from ever getting that deep in the first place -- checked via a new CSGNode::treeDepth field, set and validated (Evaluator:: setTreeDepthOrThrow, kMaxCsgTreeDepth=2000) at every site that finalizes a node's own children (buildTreeNode, evalModularCall's non-splice tail, spliceModuleChildren's union-wrap branch), not by a generate-pass or destructor-side walk (an earlier version of this fix tried exactly that and didn't work -- the crash isn't even inside generateTreeImpl). 697 tests green under both OSCAD_BYTECODE_VM on and off. Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/csg_node.hpp | 13 ++++ include/openscad_cpp_evaluator/evaluator.hpp | 45 ++++++++++++++ src/csg_resolve.cpp | 12 ++++ tests/test_csg_tree.cpp | 65 ++++++++++++++++++++ 4 files changed, 135 insertions(+) diff --git a/include/openscad_cpp_evaluator/csg_node.hpp b/include/openscad_cpp_evaluator/csg_node.hpp index 8ccfc68..65519d1 100644 --- a/include/openscad_cpp_evaluator/csg_node.hpp +++ b/include/openscad_cpp_evaluator/csg_node.hpp @@ -34,6 +34,19 @@ struct CSGNode { CSGParams params; // resolve step's plain-data output bool uncacheable = false; // set by a later phase (ManifoldCache, Phase 8) + // 1 + the deepest child's own treeDepth (1 for a leaf) -- set once, + // at construction, by whichever csg_resolve.cpp site finalizes this + // node's own `children` (buildTreeNode, evalModularCall's non-splice + // tail, spliceModuleChildren's union-wrap branch), which also checks + // it against Evaluator::kMaxCsgTreeDepth right there and throws + // before an unsafely deep tree can ever exist. See that constant's + // own doc comment (evaluator.hpp) for why this has to be enforced at + // construction time, not by a later walk over the finished tree -- + // by the time such a tree exists, std::unique_ptr's own + // default (recursive) destructor is itself an un-guardable native- + // stack risk the moment this node is ever destroyed. + int treeDepth = 1; + // Memoizes manifold_cache.cpp's cacheKey(*this) -- that function // recurses into every descendant to build its own key, so without this // memo a generate-time walk that calls cacheKey() at every node (not diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 8d898ff..fbd781b 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -547,6 +547,15 @@ class Evaluator { void buildTreeNode(const std::string& kind, const oscad::ASTNode& node, const std::function& resolveBody); + // Sets node.treeDepth from its own (already-finalized) `children`, + // throwing (kMaxCsgTreeDepth's own doc comment, above, for why) if + // it's now too deep. Called by every csg_resolve.cpp site that + // finalizes a CSGNode's own `children` -- buildTreeNode, + // evalModularCall's non-splice tail, spliceModuleChildren's + // union-wrap branch -- right after assigning `children`, before the + // node is ever linked into anything a caller could see. + void setTreeDepthOrThrow(CSGNode& node, const oscad::ASTNode& errNode); + // -- User function/module calls (Phase 4) -------------------------- public: @@ -1354,6 +1363,42 @@ class Evaluator { // Interpreted (PR #59) instead of throwing cleanly at the guard. static constexpr size_t kMaxUserCallDepth = 30; + // Cap on CSGNode::treeDepth (csg_node.hpp), checked wherever a CSGNode + // is finalized with its own `children` already known (buildTreeNode, + // evalModularCall's own non-splice tail, spliceModuleChildren's + // union-wrap branch -- csg_resolve.cpp) -- i.e. enforced DURING + // resolve, at tree-CONSTRUCTION time, not by walking the tree + // afterward. This is deliberately NOT a generateTreeImpl-side guard + // (an earlier version of this fix was exactly that, and it didn't + // work): the RESOLVE pass's own depth guards (kMaxUserCallDepth, + // kMaxVmCallStackDepth for compiled module recursion) only bound how + // many CALLS happen, not how deep the CSGNode TREE those calls build + // ends up being -- confirmed directly that resolveTree() itself + // returns successfully for a 100,000-deep incrementally-nested tree + // (`module recur(n) { if (n>0) { cube(0.1); recur(n-1); } else { + // cube(1); } }` -- an ordinary "spiral/chain of shapes" pattern, not + // exotic: each level's own module-call splice sees >1 child, so + // evalModularCall wraps them in a synthetic "union" CSGNode instead of + // collapsing away, and the tree genuinely grows one level per call). + // The crash isn't even IN generateTreeImpl's own walk -- it's in + // std::unique_ptr's default RECURSIVE destructor, freeing + // that same 100,000-deep tree the moment it goes out of scope (a + // standalone repro confirmed this directly: resolveTree() returns + // fine and prints its own result; the crash happens purely from + // letting that return value be destroyed, generateTree() never even + // called). A destructor can't throw a catchable error and a real + // stack overflow isn't a C++ exception at all -- there is no way to + // "guard" the walk or the destructor after the fact, only to stop the + // tree from ever getting that deep in the first place. Picked + // conservatively (not from a precise Windows measurement, no access + // to one locally) -- a chain 10x this depth already showed a real + // performance cliff in Manifold's own deeply-nested boolean unions + // (unrelated to this guard, but confirms nothing legitimate needs to + // go anywhere near this many levels); revisit with the same + // CI-diagnostic-loop method kMaxUserCallDepth's own history used if + // it ever needs recalibrating. + static constexpr int kMaxCsgTreeDepth = 2000; + // See lastChildrenPositions()'s own doc comment. std::optional>> lastChildrenPositions_; diff --git a/src/csg_resolve.cpp b/src/csg_resolve.cpp index eaf6870..76b17c8 100644 --- a/src/csg_resolve.cpp +++ b/src/csg_resolve.cpp @@ -28,6 +28,15 @@ class ResolvePassGuard { }; } // namespace +void Evaluator::setTreeDepthOrThrow(CSGNode& node, const oscad::ASTNode& errNode) { + int maxChildDepth = 0; + for (const auto& c : node.children) maxChildDepth = std::max(maxChildDepth, c->treeDepth); + node.treeDepth = maxChildDepth + 1; + if (node.treeDepth > kMaxCsgTreeDepth) { + error("Recursion too deep while building geometry", errNode); + } +} + void Evaluator::buildTreeNode(const std::string& kind, const oscad::ASTNode& node, const std::function& resolveBody) { // Push a fresh accumulator for whatever CSGNodes get created while @@ -60,6 +69,7 @@ void Evaluator::buildTreeNode(const std::string& kind, const oscad::ASTNode& nod treeNode->children = std::move(children); treeNode->params = std::move(params); treeNode->uncacheable = uncacheable; + setTreeDepthOrThrow(*treeNode, node); treeStack_.back().push_back(std::move(treeNode)); } @@ -128,6 +138,7 @@ void Evaluator::evalModularCall(const oscad::ModularCall& node, EvalContext& ctx treeNode->uncacheable = uncacheable; treeNode->children = std::move(children); treeNode->params = std::move(params); + setTreeDepthOrThrow(*treeNode, node); treeStack_.back().push_back(std::move(treeNode)); } @@ -150,6 +161,7 @@ void Evaluator::spliceModuleChildren(std::vector> child unionNode->isBuiltin = false; unionNode->uncacheable = std::any_of(children.begin(), children.end(), [](const auto& c) { return c->uncacheable; }); unionNode->children = std::move(children); + setTreeDepthOrThrow(*unionNode, callNode); treeStack_.back().push_back(std::move(unionNode)); } else { for (auto& c : children) treeStack_.back().push_back(std::move(c)); diff --git a/tests/test_csg_tree.cpp b/tests/test_csg_tree.cpp index 5312370..6a20d6f 100644 --- a/tests/test_csg_tree.cpp +++ b/tests/test_csg_tree.cpp @@ -7,6 +7,20 @@ using namespace oscadeval; using namespace oscadeval::test; +namespace { +// Mirrors test_bytecode_compiler.cpp's/test_tail_calls.cpp's own ScopedVm -- +// the plain OSCAD_BYTECODE_VM env var is cached forever after its first +// read, so a test whose own correctness depends on the compiled path +// actually running (see BareModuleRecursionStaysTreeDepthOneRegardlessOf +// CallDepth, below -- 20,000-deep recursion needs Op::CallModule, not +// kMaxUserCallDepth=30's native recursion) must force it explicitly. +class ScopedVm { +public: + explicit ScopedVm(bool enabled) { Evaluator::setBytecodeVmEnabledForTesting(enabled); } + ~ScopedVm() { Evaluator::setBytecodeVmEnabledForTesting(std::nullopt); } +}; +} // namespace + // -- Tree shape ----------------------------------------------------------- TEST(CsgTree, OneModularCallProducesOneTreeNode) { @@ -155,3 +169,54 @@ TEST(CsgTree, LinearExtrudeNestsItsChildUnderneath) { ASSERT_EQ(e.tree[0]->children.size(), 1u); EXPECT_EQ(e.tree[0]->children[0]->kind, "circle"); } + +// -- CSGNode::treeDepth / Evaluator::kMaxCsgTreeDepth ---------------------- +// +// A recursive user-module call that wraps only a SINGLE spliced child +// (evalModularCall's own splice branch collapses it away, no extra +// CSGNode) can recurse arbitrarily deep (bounded only by kMaxUserCallDepth/ +// kMaxVmCallStackDepth, the CALL-depth guards) without the resulting TREE +// ever getting any deeper. A recursive module that instead adds its OWN +// incremental geometry each level -- an ordinary "chain/spiral of shapes" +// pattern, not exotic -- makes each level's own splice see >1 child, so it +// gets wrapped in a synthetic "union" CSGNode instead of collapsing away, +// and the TREE genuinely grows one level per call. Before this guard, a +// long enough chain resolved fine (the call-depth guards don't bound tree +// depth at all) and then segfaulted the instant the resulting CSGNode +// tree -- built, never walked -- went out of scope: std::unique_ptr< +// CSGNode>'s own default recursive destructor is itself an un-guardable +// native-stack risk once such a tree exists at all, so this has to be +// caught DURING resolve, at tree-construction time, not by a later walk. + +TEST(CsgTree, BareModuleRecursionStaysTreeDepthOneRegardlessOfCallDepth) { + ScopedVm vm(true); // 20,000-deep recursion needs the compiled path + // Splicing collapses every level away -- 20,000 calls deep, but the + // FINAL tree is still just the one leaf cube. Proves the new guard + // (kMaxCsgTreeDepth = 2000) doesn't false-positive on the shape Stage + // 2's own deep-recursion tests already rely on. + Evaluated e = evalSrc("module recur(n) { if (n>0) recur(n-1); else cube(1); }\nrecur(20000);"); + ASSERT_EQ(e.tree.size(), 1u); + EXPECT_EQ(e.tree[0]->kind, "cube"); + EXPECT_EQ(e.tree[0]->treeDepth, 1); +} + +TEST(CsgTree, IncrementalGeometryChainHitsTheDepthGuardCleanlyInsteadOfCrashing) { + // Forced compiled specifically so this is unambiguously testing THIS + // guard (kMaxCsgTreeDepth=2000) and not incidentally passing because + // interpreted recursion would ALSO throw first, at kMaxUserCallDepth + // (30) -- a much shallower, unrelated guard. + ScopedVm vm(true); + // Each level adds its own cube(0.1) alongside the recursive call, so + // the tree grows one level per call -- 5000 is comfortably past + // kMaxCsgTreeDepth (2000). Before this guard, a script shaped exactly + // like this segfaulted (confirmed directly: resolveTree() itself + // returned successfully and printed its own result; the crash was + // purely from destroying the resulting tree afterward, generateTree() + // never even reached). + try { + evalSrc("module recur(n) { if (n>0) { cube(0.1); recur(n-1); } else { cube(1); } }\nrecur(5000);"); + FAIL() << "expected EvalError"; + } catch (const EvalError& e) { + EXPECT_NE(std::string(e.what()).find("Recursion too deep"), std::string::npos); + } +} From 62f1d62cad4f04c3328614c536360c6b3a19e8e3 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 10:25:31 -0700 Subject: [PATCH 5/5] Bound TRACE output for genuinely deep non-tail call chains A tail-recursive chain mutates one CallStackFrame in place per hop, so its trace was already small regardless of depth. A non-tail chain has no such collapsing -- every call is a real, distinct frame -- and Stage 1/2's explicit-stack VM means that chain can now legitimately run thousands deep. traceLines() printed one "TRACE:" pair per frame unconditionally, turning a single error into a multi-megabyte message (confirmed directly: the CSG-tree-depth guard's own error, PR #59). Now shows the innermost 10 frames (closest to the actual error) and outermost 10 (the original entry point), with one "... N more frames ..." marker in between once the stack exceeds 20 frames -- unchanged for every existing shallow trace. Deliberate departure from the Python reference (traceLines mirrors its own _trace_lines): Python's recursion limit makes a callStack this deep unreachable there. 698 tests green under both OSCAD_BYTECODE_VM on and off. Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/eval_error.hpp | 9 +++++ src/eval_error.cpp | 33 ++++++++++++++++++- tests/test_csg_tree.cpp | 24 ++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/include/openscad_cpp_evaluator/eval_error.hpp b/include/openscad_cpp_evaluator/eval_error.hpp index 1013f29..5b4b811 100644 --- a/include/openscad_cpp_evaluator/eval_error.hpp +++ b/include/openscad_cpp_evaluator/eval_error.hpp @@ -95,6 +95,15 @@ std::string locSuffix(const oscad::Position* pos); // Module: "TRACE: call of '()'" // "TRACE: called by ''" // Function: "TRACE: called by ''" +// One deliberate departure from the reference: once callStack itself has +// more than a small number of frames, only the innermost and outermost +// few are shown, with a single "... N more frames ..." marker in +// between -- see traceLines()'s own definition (eval_error.cpp) for why. +// The reference never needed this (Python's own recursion limit makes a +// callStack this deep unreachable there); this port's explicit-stack VM +// (Stage 1/2) can legitimately recurse thousands of calls deep, and +// printing one line per frame turned a single error into a +// multi-megabyte message. std::vector traceLines(const oscad::Position* nodePosition, const std::vector& callStack, const std::string& innermostFrame = ""); diff --git a/src/eval_error.cpp b/src/eval_error.cpp index 9a0cebb..b6e2a7c 100644 --- a/src/eval_error.cpp +++ b/src/eval_error.cpp @@ -26,7 +26,38 @@ std::vector traceLines(const oscad::Position* nodePosition, if (!innermostFrame.empty()) { lines.push_back("TRACE: called by '" + innermostFrame + "'" + locSuffix(nodePosition)); } - for (auto it = callStack.rbegin(); it != callStack.rend(); ++it) { + + // A tail-recursive chain mutates ONE CallStackFrame in place per hop + // (recordTailCallHop, user_calls.cpp), so callStack itself never + // reflects how many hops actually happened -- its trace is already + // small regardless of depth (see TailCalls. + // ErrorThrownDeepInATailChainProducesABoundedTrace). A genuinely + // non-tail recursive chain has no such collapsing: every call is a + // real, distinct CallStackFrame, and Stage 1/2's own explicit-stack + // VM means that chain can legitimately run thousands of frames deep + // now (confirmed reachable, not hypothetical: CsgTree. + // IncrementalGeometryChainHitsTheDepthGuardCleanlyInsteadOfCrashing + // produced a multi-megabyte error message before this bound existed). + // Shows the innermost kFramesEachEnd frames (closest to the actual + // error -- generally the most useful for debugging) and the + // outermost kFramesEachEnd (the original entry point, for overall + // context), with one marker line summarizing how many were skipped + // in between -- the same shape most language tracebacks use for a + // pathologically deep stack. + constexpr size_t kFramesEachEnd = 10; + const size_t total = callStack.size(); + const bool truncate = total > 2 * kFramesEachEnd; + + size_t idx = 0; + for (auto it = callStack.rbegin(); it != callStack.rend(); ++it, ++idx) { + if (truncate && idx >= kFramesEachEnd && idx < total - kFramesEachEnd) { + if (idx == kFramesEachEnd) { + const size_t skipped = total - 2 * kFramesEachEnd; + lines.push_back("TRACE: ... " + std::to_string(skipped) + " more frame" + (skipped == 1 ? "" : "s") + + " ..."); + } + continue; + } const CallStackFrame& frame = *it; if (frame.kind == CallStackFrame::Kind::Module) { lines.push_back("TRACE: call of '" + frame.name + "()'" + locSuffix(frame.declPosition)); diff --git a/tests/test_csg_tree.cpp b/tests/test_csg_tree.cpp index 6a20d6f..ebcd5e1 100644 --- a/tests/test_csg_tree.cpp +++ b/tests/test_csg_tree.cpp @@ -220,3 +220,27 @@ TEST(CsgTree, IncrementalGeometryChainHitsTheDepthGuardCleanlyInsteadOfCrashing) EXPECT_NE(std::string(e.what()).find("Recursion too deep"), std::string::npos); } } + +TEST(CsgTree, DeepNonTailChainErrorTraceStaysBoundedInsteadOfMegabytes) { + // Same shape as IncrementalGeometryChainHitsTheDepthGuardCleanlyInstead + // OfCrashing, above, but asserting on the TRACE itself: before + // traceLines() (eval_error.cpp) bounded it, this exact error carried + // one "TRACE:" pair per real (non-tail) call stack frame -- ~6000 + // lines, several megabytes, for a script that isn't doing anything + // unusual by this session's own new standards (Stage 1/2 make exactly + // this depth of real recursion routine). Checks the marker line + // appears and the total TRACE-line count stays small regardless of + // how deep the actual chain was. + ScopedVm vm(true); + try { + evalSrc("module recur(n) { if (n>0) { cube(0.1); recur(n-1); } else { cube(1); } }\nrecur(5000);"); + FAIL() << "expected EvalError"; + } catch (const EvalError& e) { + const std::string msg = e.what(); + EXPECT_NE(msg.find("TRACE: ... "), std::string::npos); + EXPECT_NE(msg.find(" more frames ..."), std::string::npos); + size_t count = 0; + for (size_t pos = msg.find("TRACE:"); pos != std::string::npos; pos = msg.find("TRACE:", pos + 1)) ++count; + EXPECT_LE(count, 41u); // 40 shown (20 each end, 2 lines/Module frame) + 1 marker + } +}