From 5982da991495d379eec07d057a3edb8682755002 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 15:26:52 -0700 Subject: [PATCH 1/3] Extend depth-safe compiled path to modifiers; compile leaf statements; fix two real crash bugs found by BOSL2 sweep Item 1: evalModifier now routes its wrapped child through evalChildren instead of a hand-rolled checkDebug+evalStatement pair, so a `#`/`!`- wrapped recursive module call gets the same Op::CallModule depth treatment translate()/union() already have from the NativeStatement-gap fix. intersection_for/let() turned out to already be covered (both already call evalChildren internally). Item 2: Assignment/ModularEcho/ModularAssert/ModularLet now compile to real bytecode instead of falling back to Op::NativeStatement. This closes a real regression: compileOneStatement had no Assignment case at all, so the whole-list compile added for the NativeStatement gap was silently shadowing the older, more specialized tryRunCompiledAssignmentBlock path. New opcodes: CheckDebugStatement, StoreModuleVar, AssertStatement (+ AssertSite, named-arg + eager-evaluation support matching evalAssertStatement's contract, distinct from AssertOp's lazy-message one), OpenLetScope/StoreLetVar. Module chunks can now have nonzero numSlots (a nested let-expression inside one of these statements' own arguments needs real slots, unlike anything else in a module body). Landing item 2 surfaced and fixed two real bugs, both caught by the existing test suite: a `$fn` override leaking past its own let() scope into later statements (inlining sub-expressions bypassed the isolation runCompiledExprChunk used to provide -- fixed with Op::OpenExprScope/ CloseExprScope), and function calls inside these new inline expressions resolving against the wrong lexical scope (compileExpr's PrimaryCall case reads the Compiler's fixed scope_ member directly, not per-statement -- fixed with an RAII scope swap in compileOneStatement). A differential sweep of BOSL2's own test corpus (901 tests, VM on/off) then found two further bugs, unrelated to items 1/2, both confirmed present on the pre-existing PR #60 baseline via git stash before fixing: - kMaxDriveVmNativeDepth=30 was too conservative for real BOSL2 usage -- 3 scripts (test_ball_bearing, test_teardrop_corner_mask, test_rounding_hole_mask) that used to render now failed via BOSL2's own deep _translate()/_show_highlight() wrapper nesting. Empirically raised to 100 (binary-searched: 50 is the minimum that fixes all 3, doubled for headroom) -- re-verified the original crash-safety guarantee still holds at the new value. - A real memory-corruption crash: test_poly_roots segfaulted after printing a fully correct error message. Root-caused via a targeted AddressSanitizer build to Evaluator::releaseVmFrame never resetting ownsModuleSplice/moduleRandsBefore/moduleSpliceCallNode before returning a VmFrame to the pool -- a frame released after owning a module's own splice, later reused (LIFO) for an unrelated non-tail recursive function call, kept ownsModuleSplice=true; when that function call was torn down by an exception thrown mid-recursion, teardownVmCallStackDownTo popped treeStack_ once too many, corrupting it. Fixed by resetting all three fields in releaseVmFrame, matching the reset it already does for stack/slots/bound. Full 708-test suite green both OSCAD_BYTECODE_VM states; BOSL2 differential sweep clean (zero crashes across all 901 tests, down from 2 before this commit). Version bumped 0.12.0 -> 0.13.0 per repo convention. Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/bytecode.hpp | 159 +++++++++++++-- include/openscad_cpp_evaluator/evaluator.hpp | 27 ++- pyproject.toml | 2 +- src/bytecode_compiler.cpp | 200 ++++++++++++++++++- src/bytecode_vm.cpp | 111 +++++++++- src/csg_resolve.cpp | 26 ++- src/user_calls.cpp | 22 ++ tests/test_bytecode_compiler.cpp | 103 ++++++++++ 8 files changed, 603 insertions(+), 47 deletions(-) diff --git a/include/openscad_cpp_evaluator/bytecode.hpp b/include/openscad_cpp_evaluator/bytecode.hpp index d58ffbe..94f1d41 100644 --- a/include/openscad_cpp_evaluator/bytecode.hpp +++ b/include/openscad_cpp_evaluator/bytecode.hpp @@ -14,6 +14,7 @@ class FunctionDeclaration; class FunctionLiteral; class ModuleDeclaration; class ModularCall; +class ModularAssert; class Expression; } // namespace oscad @@ -259,23 +260,26 @@ enum class Op { // 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 single "native passthrough" statement -- a 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 statically known) -- anything + // compileStatementList doesn't give its own real bytecode. (Assignment/ + // ModularEcho/ModularAssert/ModularLet used to fall here too; they now + // have their own real bytecode -- Op::StoreModuleVar/Op::Echo/ + // Op::AssertStatement/Op::OpenLetScope+StoreLetVar -- purely a + // throughput change, since none of these were ever the recursion-depth + // risk this compiler targets.) // 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), + // for one node: derive childCtx via ctx.withScope(...), checkDebug, // 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. + // resolve function walking ITS children, 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:: @@ -334,6 +338,108 @@ enum class Op { // ForIterNext instruction to attempt the next one. a = jump target // (the matching ForIterNext's own pc). ForIterEnd, + + // -- Module-body leaf-statement compilation (throughput, not a -------- + // -- recursion-safety concern -- see NativeStatement's own doc comment + // for why THAT stayed native for these same node kinds in Stage 2; + // this closes the "every sub-expression is a separate nested driveVm + // call via evalExprMaybeCompiled" gap instead by inlining them into + // the enclosing chunk's own instruction stream) -------------------- + // + // A module-body statement's own top-level checkDebug -- the compiled + // analog of what evalChildren's native per-statement loop / Op:: + // NativeStatement's own handler already does for the SAME node + // (`ev.checkDebug(*stmt, ctx)`, exprLevel=false, forced=false) -- kept + // as a real op (not simply omitted) so a compiled statement's debug + // behavior is identical to its native form, not just "usually doesn't + // matter because fast-continue only compiles breakpoint-free chunks." + // a = index into CompiledChunk::nativeStatements (reusing that table). + CheckDebugStatement, + + // Assignment's own compiled form -- pops one Value (already evaluated + // via an ordinary inline compileExpr, NOT native evalExprMaybeCompiled) + // and replicates Evaluator::evalAssignment exactly: a `$`-prefixed name + // goes to ctx.dyn (+ dynExplicit), everything else checks + // ctx.dynPositions for the "assigned but overwritten" warning, then + // writes ctx.let_ and records this position. Never slot-addressed -- + // module-level/top-level assignments are always ctx-visible, dynamic- + // scope writes, exactly like today. a = name pool index, pos = the + // Assignment node's own position (for both the overwritten warning and + // the position recorded into dynPositions). + StoreModuleVar, + + // ModularEcho/ModularAssert's own statement form share Op::Echo/a + // dedicated AssertStatement op (below) for the "pop N already-compiled + // argument values" mechanics -- see those ops' own doc comments + // (Op::Echo, above; Op::AssertStatement, below) for why the + // EXPRESSION-form opcodes are directly reusable (statement-shaped + // already: no value pushed, straight fall-through) while assert's + // needs its own op (named-argument support + eager-not-lazy message + // evaluation, both genuinely different from AssertOp's own contract). + + // Isolates ONE inline-compiled sub-expression's own `$`-writes from the + // rest of this chunk's shared, long-lived ctx -- pushes ctx. + // letChildCtx() (reads still see through to the parent level; only + // WRITES are isolated) onto f.ctxChain, mirroring exactly what + // runCompiledExprChunk's own wrapper already does for a native call + // into a compiled EXPRESSION chunk (evalExprMaybeCompiled). Needed + // because a module-body statement's own inline-compiled sub- + // expression (Assignment's RHS, one echo()/assert() argument) has NO + // such wrapper of its own the way a genuinely separate chunk call + // does -- without this, `v1 = let($fn=55) f(); v2 = f();` leaks $fn=55 + // into v2 too (caught for real by UserFunction. + // DollarVarLetAsAssignmentRhsDoesNotLeak once Assignment got its own + // inline compiled form). Always emitted, unconditionally, around + // EVERY such sub-expression -- not just ones statically known to + // contain a `let($...)` -- for the same reason runCompiledExprChunk's + // own wrapper is unconditional: a nested call/closure could still + // reach one indirectly, and the cost when nothing $-scoped is present + // is just one cheap trail-level open, not a real expression re-walk. + OpenExprScope, + // Pops the ctx OpenExprScope just pushed (f.ctxChain.pop_back()) -- + // always paired, immediately after the sub-expression's own compiled + // code finishes (its RESULT, if any, is already on f.stack by then; + // this only tears down the ctx, never touches the value stack). Also + // reused to close Op::OpenLetScope's own child ctx (below) -- both are + // just "pop the current ctx level", identical either way. + CloseExprScope, + + // ModularLet's own (statement-form) child scope -- mirrors + // Evaluator::evalLetBlock's own `ctx.childCtx(nullptr, std::nullopt, + // ctx.childrenNodes, ctx.childrenCallerCtx)` call exactly, pushed onto + // f.ctxChain (Op::CloseExprScope, above, pops it back off once the + // let-block's own body finishes compiling/running). Every assignment's + // own RHS is compiled+evaluated BEFORE this runs (while the PARENT ctx + // is still current -- see ModularLet's own compileOneStatement case, + // bytecode_compiler.cpp, for why: the statement form's RHS must never + // see an EARLIER sibling's own write in the SAME let-block, unlike the + // let-EXPRESSION form's sequential visibility), each left on f.stack; + // only the WRITES (Op::StoreLetVar, below) happen after this runs. + OpenLetScope, + + // Op::StoreLetVar -- pops one value and writes it into the CURRENT ctx + // (by then, the child Op::OpenLetScope just pushed) via the same + // `$`-prefix branch Op::StoreModuleVar uses, but WITHOUT its + // dynPositions "assigned but overwritten" bookkeeping -- evalLetBlock's + // own writes never do that check either (a let-block's own child scope + // is always fresh, so "reassigning the same name in the same scope" + // can never happen the way a module-body Assignment's own can). a = + // name pool index, pos = the Assignment node's own position (unused by + // the handler itself, kept for consistency/future debugging only). + StoreLetVar, + + // a = index into CompiledChunk::assertSites. Pops site.argCount values + // (all of this statement's own arguments, already compiled+pushed in + // source order via compileExpr -- EVERY one, not just condition/ + // message, mirrors evalAssertStatement's own eager resolveArgs() over + // AssertOp's lazy-message contract) and replicates + // Evaluator::evalAssertStatement exactly: named "condition"/"message" + // win over positional 0/1 (resolved at COMPILE time into + // AssertSite::conditionArgIndex/messageArgIndex -- the argument SHAPE + // is static, only values vary per call), throws via Evaluator::error() + // on failure, otherwise runs any chained node.children natively (rare; + // not worth its own compiled path -- see AssertSite's own doc comment). + AssertStatement, }; struct Instruction { @@ -477,6 +583,32 @@ struct CompiledChunk { std::string calleeName; }; + // One Op::AssertStatement site -- see that op's own doc comment for + // the full contract. `conditionArgIndex`/`messageArgIndex` are indices + // into the site's own argCount-sized popped-argument array (source + // order), resolved at COMPILE time via the same "named wins over + // positional 0/1" priority Evaluator::getArg applies at runtime -- + // nullopt means "no argument supplies this logical parameter" (mirrors + // getArg's own defaultValue fallback: absent condition reads as + // `Value{true}`, absent message means no ": ..." suffix). + // `condTextConstIdx`: constants[] index of the condition argument's own + // source text (or "false" when no condition argument exists, matching + // evalAssertStatement's `node.arguments.empty() ? "false" : ...`), + // interned once at compile time. `node`: for both Evaluator::error()'s + // TRACE walk and the (rare) chained-children fallback below. + // + // ponytail: a script with TWO named args of the SAME name (e.g. + // `assert(condition=true, condition=false)`) would resolve to the + // LAST one here, not getArg's own first-match; not worth the extra + // bookkeeping to fix for input that's already nonsensical. + struct AssertSite { + int argCount = 0; + std::optional conditionArgIndex; + std::optional messageArgIndex; + int condTextConstIdx = -1; + const oscad::ModularAssert* node = nullptr; + }; + // 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 @@ -512,6 +644,7 @@ struct CompiledChunk { // NativeCondJumpIfFalse/NativeCheckDebugExprLevel/NativeIterMaterialize's // own doc comments, above) -- always empty for a function chunk. std::vector moduleCallSites; + std::vector assertSites; std::vector nativeExprs; std::vector nativeStatements; int numSlots = 0; diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 6521ec3..af0999a 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -1336,14 +1336,25 @@ class Evaluator { // free function, not a member, so it needs direct access with no // friend declaration in play. size_t driveVmNativeDepth_ = 0; - // Same figure as kMaxUserCallDepth -- each native re-entry above costs - // a comparable handful of native frames (evalStatement, evalModularCall, - // a builtin resolve function, evalChildren, tryRunCompiledChildren, - // runCompiledModuleBody, driveVm), so the same Windows-CI-calibrated - // ceiling applies. Deliberately its own named constant (not a reuse of - // kMaxUserCallDepth in place) so the two can be recalibrated - // independently if profiling ever shows they should diverge. - static constexpr size_t kMaxDriveVmNativeDepth = 30; + // Originally set to kMaxUserCallDepth's own figure (30) on the + // assumption the two native-reentry chains cost a comparable handful + // of frames each -- WRONG in practice: a real differential sweep + // against BOSL2's own test corpus found 3 genuine scripts + // (test_ball_bearing, test_teardrop_corner_mask, test_rounding_hole_ + // mask -- all built on deeply layered _translate()/_show_highlight()/ + // attachment-wrapper composition, not runaway recursion) that used to + // render successfully but started failing with "Recursion too deep + // (native call stack)" once this guard shipped at 30. Empirically + // raised to the smallest value (binary search, local rebuild+retest) + // that lets all 3 succeed again (50), then doubled for headroom + // against future library growth -- NOT independently re-verified + // against Windows CI's own tighter stack margin the way + // kMaxUserCallDepth's own 30 was (that number came DOWN from an + // initial 50 specifically because 50 segfaulted there) -- if CI ever + // flags this as unsafe on a real run, come back down, don't guess + // again locally. Deliberately its own named constant (not a reuse of + // kMaxUserCallDepth) so the two can be recalibrated independently. + static constexpr size_t kMaxDriveVmNativeDepth = 100; // Bracketed public in place: Op::CallModule's own runtime handler and // driveVm's module-frame completion branch (bytecode_vm.cpp, a diff --git a/pyproject.toml b/pyproject.toml index cd58756..7436864 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.12.0" +version = "0.13.0" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/bytecode_compiler.cpp b/src/bytecode_compiler.cpp index caad082..7fbb790 100644 --- a/src/bytecode_compiler.cpp +++ b/src/bytecode_compiler.cpp @@ -1150,9 +1150,46 @@ class Compiler { for (const oscad::ASTNode* stmt : others) compileOneStatement(*stmt, out); } + // Compiles one inline sub-expression wrapped in Op::OpenExprScope/ + // CloseExprScope -- see those ops' own doc comment (bytecode.hpp) for + // why every module-body statement's own inline-compiled RHS/argument + // needs this (a `$`-write leak this session's own regression test, + // UserFunction.DollarVarLetAsAssignmentRhsDoesNotLeak, caught for + // real). Shared by Assignment/ModularEcho/ModularAssert's own + // compileOneStatement cases, below. + void compileIsolatedExpr(const oscad::Expression& expr, std::vector& out, CompileScope& scope) { + out.push_back({Op::OpenExprScope, 0, 0, nullptr}); + compileExpr(expr, out, scope); + out.push_back({Op::CloseExprScope, 0, 0, nullptr}); + } + void compileOneStatement(const oscad::ASTNode& stmt, std::vector& out) { using oscad::NodeKind; trackSpan(stmt); + // A statement's own .scope() can differ from this whole chunk's + // fixed scope_ member (e.g. a `use` statement earlier in the SAME + // script/module body changes what later statements can see) -- + // compileExpr's own PrimaryCall case resolves a callee via scope_ + // DIRECTLY, not per-node, unlike the ModularCall case just below + // (which already does its own stmt.scope()-first lookup). Any + // inline sub-expression compiled from Assignment/ModularEcho/ + // ModularAssert, below, needs scope_ swapped to match THIS + // statement for its own duration, or a nested function call inside + // it resolves against the wrong (chunk-wide) scope instead of its + // own. Caught for real: UseStatement.NestedUseNotReExported + // started spuriously resolving a NOT-re-exported nested `use`'s + // function once echo() got its own inline-compiled argument. + // RAII (not "restore before every return") since NotCompilable can + // unwind through here -- harmless either way (the whole Compiler + // is discarded on that path), but cheap to get right regardless. + struct ScopeGuard { + const oscad::Scope*& scope; + const oscad::Scope* saved; + ScopeGuard(const oscad::Scope*& s, const oscad::ASTNode& node) : scope(s), saved(s) { + if (const oscad::Scope* stmtScope = node.scope()) scope = stmtScope; + } + ~ScopeGuard() { scope = saved; } + } scopeGuard(scope_, stmt); switch (stmt.kind()) { // Pure declarations, no-ops at statement-eval time (already // hoisted into scope by buildScopes()) -- matches evalStatement's @@ -1160,6 +1197,145 @@ class Compiler { case NodeKind::ModuleDeclaration: case NodeKind::FunctionDeclaration: return; + // Assignment/ModularEcho/ModularAssert: real bytecode instead + // of Op::NativeStatement -- a throughput improvement (see + // CheckDebugStatement/StoreModuleVar/AssertStatement's own doc + // comments, bytecode.hpp), NOT a recursion-safety one (these + // were never the risk Stage 2 targeted -- see NativeStatement's + // own doc comment). Assignment specifically closes a real + // regression: it used to get a genuinely optimized path + // (Evaluator::tryRunCompiledAssignmentBlock) via evalChildren's + // own native fallback loop, but tryRunCompiledChildren's + // broader whole-list compile (the NativeStatement-gap fix) + // ALWAYS succeeds first now, silently shadowing it -- every + // assignment fell back to generic Op::NativeStatement, one + // nested driveVm call per sub-expression via + // evalExprMaybeCompiled, instead of this inline form. + case NodeKind::Assignment: { + auto& n = static_cast(stmt); + out.push_back({Op::CheckDebugStatement, internNativeStatement(&stmt), 0, nullptr}); + CompileScope exprScope; // module-level vars are never slot-addressed -- + // see StoreModuleVar's own doc comment. + compileIsolatedExpr(*n.expr, out, exprScope); + out.push_back({Op::StoreModuleVar, internName(n.name->name), 0, &n.position()}); + return; + } + case NodeKind::ModularEcho: { + // Op::Echo itself is reused verbatim from the EXPRESSION + // form's own compile case (AssertOp's sibling, above in + // compileExpr) -- already statement-shaped (no value + // pushed, straight fall-through), so nothing new is needed + // there. `.children` is deliberately never read here, + // matching evalStatement's own ModularEcho case (doEcho + // only ever takes `.arguments` -- an echo statement's + // trailing children, if the grammar even produces any, are + // not evaluated today; this mirrors that exactly rather + // than silently changing behavior). + auto& n = static_cast(stmt); + out.push_back({Op::CheckDebugStatement, internNativeStatement(&stmt), 0, nullptr}); + CompiledChunk::EchoSite site; + CompileScope exprScope; + for (const auto& argPtr : n.arguments) { + if (argPtr->kind() == NodeKind::NamedArgument) { + auto& a = static_cast(*argPtr); + compileIsolatedExpr(*a.expr, out, exprScope); + site.argNames.push_back(a.name->name); + } else { + auto& a = static_cast(*argPtr); + compileIsolatedExpr(*a.expr, out, exprScope); + site.argNames.push_back(std::nullopt); + } + } + int siteIdx = static_cast(chunk_.echoSites.size()); + chunk_.echoSites.push_back(std::move(site)); + out.push_back({Op::Echo, siteIdx, static_cast(n.arguments.size()), &n.position()}); + return; + } + case NodeKind::ModularAssert: { + // Genuinely different contract from AssertOp's own compiled + // form (Op::AssertFail): the statement form supports named + // arguments AND evaluates every argument EAGERLY (mirrors + // Evaluator::evalAssertStatement's own resolveArgs() call + // exactly -- unlike AssertOp's lazily-compiled message, + // guarded by a runtime JumpIfTrue), so every argument is + // compiled+pushed here unconditionally, in source order. + auto& n = static_cast(stmt); + out.push_back({Op::CheckDebugStatement, internNativeStatement(&stmt), 0, nullptr}); + CompiledChunk::AssertSite site; + site.node = &n; + site.argCount = static_cast(n.arguments.size()); + CompileScope exprScope; + for (size_t i = 0; i < n.arguments.size(); ++i) { + compileIsolatedExpr(*argExpr(*n.arguments[i]), out, exprScope); + if (n.arguments[i]->kind() == NodeKind::NamedArgument) { + auto& na = static_cast(*n.arguments[i]); + if (na.name->name == "condition") site.conditionArgIndex = static_cast(i); + else if (na.name->name == "message") site.messageArgIndex = static_cast(i); + } + } + // Second pass: positional 0/1 fill whichever logical + // parameter a named arg didn't already claim -- run AFTER + // the named pass above (not interleaved) so a named arg + // wins regardless of its position relative to its + // positional counterpart in source, matching + // Evaluator::getArg's own "named first" priority exactly. + int posCounter = 0; + for (size_t i = 0; i < n.arguments.size(); ++i) { + if (n.arguments[i]->kind() == NodeKind::NamedArgument) continue; + if (posCounter == 0 && !site.conditionArgIndex) site.conditionArgIndex = static_cast(i); + else if (posCounter == 1 && !site.messageArgIndex) site.messageArgIndex = static_cast(i); + ++posCounter; + } + // Precomputed once, like AssertFail's own condText -- + // mirrors evalAssertStatement's `node.arguments.empty() ? + // "false" : argExpr(*node.arguments[0])->toString()` + // exactly (a MISSING condition arg -- e.g. a bare + // `assert(message="x");` -- reads the same as an EMPTY + // arg list here: no arg supplies "condition" either way). + site.condTextConstIdx = internConst(Value{ + site.conditionArgIndex + ? argExpr(*n.arguments[static_cast(*site.conditionArgIndex)])->toString() + : std::string("false")}); + int siteIdx = static_cast(chunk_.assertSites.size()); + chunk_.assertSites.push_back(std::move(site)); + out.push_back({Op::AssertStatement, siteIdx, 0, &n.position()}); + return; + } + case NodeKind::ModularLet: { + // Mirrors Evaluator::evalLetBlock exactly (stmt_eval.cpp): + // every assignment's RHS is evaluated against the + // ORIGINAL (parent) ctx, never an earlier sibling's own + // write in THIS SAME let-block (a documented, deliberate + // divergence from the let-EXPRESSION form's sequential + // visibility) -- so every RHS is compiled+evaluated FIRST, + // all left on f.stack, and only written into the freshly- + // opened child scope (Op::OpenLetScope) AFTER every one of + // them has already run against the still-current PARENT + // ctx. No outer statement-level check for the let-block + // node itself (matches evalChildren's own ModularLet + // exclusion) -- each assignment gets its own instead, + // exactly like evalLetBlock's own per-assignment + // checkDebug. The body (n.children) compiles inline, + // recursively, against the now-current child ctx -- same + // Compiler instance, so nested statements get every bit of + // this same real-bytecode treatment too. + auto& n = static_cast(stmt); + CompileScope exprScope; + for (const auto& assign : n.assignments) { + out.push_back({Op::CheckDebugStatement, internNativeStatement(assign.get()), 0, nullptr}); + compileIsolatedExpr(*assign->expr, out, exprScope); + } + out.push_back({Op::OpenLetScope, 0, 0, nullptr}); + // Values pop in REVERSE push order (LIFO) -- iterate the + // assignment list backwards so each Op::StoreLetVar is + // paired with the value that was actually pushed for IT. + for (auto it = n.assignments.rbegin(); it != n.assignments.rend(); ++it) { + out.push_back({Op::StoreLetVar, internName((*it)->name->name), 0, &(*it)->position()}); + } + compileStatementList(n.children, out); + out.push_back({Op::CloseExprScope, 0, 0, nullptr}); + return; + } case NodeKind::ModularCall: { auto& call = static_cast(stmt); const oscad::Scope* lookupScope = stmt.scope() ? stmt.scope() : scope_; @@ -1213,9 +1389,10 @@ class Compiler { return; } default: - // echo/assert/let-block/modifiers/intersection_for -- - // deliberately native, see this section's own header - // comment above. + // modifiers/intersection_for -- deliberately native (see + // this section's own header comment above); echo/assert/ + // assignment/let-block now have their own real bytecode + // cases, above. out.push_back({Op::NativeStatement, internNativeStatement(&stmt), 0, &stmt.position()}); return; } @@ -1405,11 +1582,16 @@ std::optional tryCompileModuleBody(const oscad::ModuleDeclaration 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. + // No params/defaultCode 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. numSlots CAN still be nonzero though: a + // nested let-EXPRESSION inside a compiled echo/assert/assignment + // argument (Assignment/ModularEcho/ModularAssert/ModularLet's own + // compileOneStatement cases) reuses LetOp's existing slot-allocating + // compile path, just scoped to that one statement's own sub- + // expression -- 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); @@ -1417,6 +1599,7 @@ std::optional tryCompileModuleBody(const oscad::ModuleDeclaration return std::nullopt; } chunk.numIterLists = compiler.nextIterList(); + chunk.numSlots = compiler.nextSlot(); return chunk; } @@ -1439,6 +1622,7 @@ std::optional tryCompileChildrenList(const std::vectorchunk = &chunk; frame->code = &chunk.bodyCode; frame->pc = 0; - frame->slots.clear(); + // See runCompiledModuleBody's own doc comment (below) for why this is + // .assign(numSlots) now, not .clear() -- a nested let-expression + // inside a compiled echo/assert/assignment argument can need local + // slots even though module parameters themselves never do. + frame->slots.assign(static_cast(chunk.numSlots), Value{}); frame->stack.clear(); frame->bound.clear(); frame->accumStack.clear(); @@ -878,14 +882,101 @@ Value driveVm(Evaluator& ev, size_t floor) { 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); + // Mirrors evalChildren's own per-statement loop exactly + // -- no ModularLet exclusion needed here (unlike that + // loop's own): ModularLet has its own compiled form now + // (Op::OpenLetScope/StoreLetVar) and never reaches + // Op::NativeStatement at all. + ev.checkDebug(*stmt, childCtx); ev.evalStatement(*stmt, childCtx); ++f.pc; break; } + case Op::OpenExprScope: { + f.ctxChain.push_back(ctx.letChildCtx()); + ++f.pc; + break; + } + case Op::CloseExprScope: { + f.ctxChain.pop_back(); + ++f.pc; + break; + } + case Op::OpenLetScope: { + f.ctxChain.push_back(ctx.childCtx(nullptr, std::nullopt, ctx.childrenNodes, ctx.childrenCallerCtx)); + ++f.pc; + break; + } + case Op::StoreLetVar: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + const std::string& name = f.chunk->names[static_cast(ins.a)]; + if (!name.empty() && name[0] == '$') { + ctx.dyn->set(name, std::move(v)); + ctx.dynExplicit->set(name, true); + } else { + ctx.let_->set(name, std::move(v)); + } + ++f.pc; + break; + } + case Op::CheckDebugStatement: { + const oscad::ASTNode* stmt = f.chunk->nativeStatements[static_cast(ins.a)]; + ev.checkDebug(*stmt, ctx); + ++f.pc; + break; + } + case Op::StoreModuleVar: { + Value v = std::move(f.stack.back()); + f.stack.pop_back(); + const std::string& name = f.chunk->names[static_cast(ins.a)]; + // Mirrors Evaluator::evalAssignment exactly (stmt_eval.cpp) -- + // see StoreModuleVar's own doc comment (bytecode.hpp). + if (!name.empty() && name[0] == '$') { + ctx.dyn->set(name, std::move(v)); + ctx.dynExplicit->set(name, true); + } else { + if (const oscad::Position* const* firstPosEntry = ctx.dynPositions->find(name)) { + const oscad::Position* firstPos = *firstPosEntry; + const int firstLine = firstPos ? firstPos->line : 0; + ev.warn(name + " was assigned on line " + std::to_string(firstLine) + " but was overwritten", + ins.pos); + } + ctx.let_->set(name, std::move(v)); + ctx.dynPositions->set(name, ins.pos); + } + ++f.pc; + break; + } + case Op::AssertStatement: { + const CompiledChunk::AssertSite& site = f.chunk->assertSites[static_cast(ins.a)]; + std::vector args(static_cast(site.argCount)); + for (int i = site.argCount - 1; i >= 0; --i) { + args[static_cast(i)] = std::move(f.stack.back()); + f.stack.pop_back(); + } + const Value condVal = site.conditionArgIndex ? std::move(args[static_cast(*site.conditionArgIndex)]) + : Value{true}; + if (!truthy(condVal)) { + std::string err = "Assertion '" + + std::get(f.chunk->constants[static_cast(site.condTextConstIdx)]) + + "' failed"; + if (site.messageArgIndex) { + const Value& msgArg = args[static_cast(*site.messageArgIndex)]; + if (!std::holds_alternative(msgArg)) { + const std::string* s = std::get_if(&msgArg); + err += ": \"" + (s ? *s : fmtValue(msgArg)) + "\""; + } + } + ev.error(err, *site.node, "assert"); + } + // Rare (assert(...) translate(...) children();-shaped) -- + // not worth its own compiled path; evalChildren already + // tries ITS OWN compiled fast path internally regardless. + if (!site.node->children.empty()) ev.evalChildren(site.node->children, ctx); + ++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)); @@ -1038,7 +1129,15 @@ void runCompiledModuleBody(Evaluator& ev, const CompiledChunk& chunk, EvalContex frame->chunk = &chunk; frame->code = &chunk.bodyCode; frame->pc = 0; - frame->slots.clear(); + // .assign, not .clear() -- a module chunk's own bodyCode has no + // PARAMETER slots (module params are always bound natively, see this + // function's own doc comment above), but CAN still declare LOCAL slots + // via a nested let-EXPRESSION inside a compiled echo/assert/assignment + // argument (Op::LetOp's own compile case, reused as-is for these -- + // see compileOneStatement's Assignment/ModularEcho/ModularAssert/ + // ModularLet cases). chunk.numSlots is 0 for the (still-common) case + // where nothing inside this body ever does that. + frame->slots.assign(static_cast(chunk.numSlots), Value{}); frame->stack.clear(); frame->bound.clear(); frame->accumStack.clear(); diff --git a/src/csg_resolve.cpp b/src/csg_resolve.cpp index b462625..035c301 100644 --- a/src/csg_resolve.cpp +++ b/src/csg_resolve.cpp @@ -175,17 +175,21 @@ void Evaluator::evalModifier(const oscad::ModuleInstantiation& child, const osca // always empty (role tagging happens entirely in the matching // generate function -- generateHighlight/Background/ShowOnly). buildTreeNode(kind, wrapperNode, [&]() { - // The wrapped child gets its own statement-level stop, in addition - // to the modifier node's (which evalChildren already fired before - // dispatching here) -- so `#cube(1);` pauses twice, like the - // reference. There, _resolve_modifier_child has no _check_debug of - // its own: it calls _eval_statement(node.child, ctx), and _that_ - // dispatcher independently checks every _TREE_NODE_TYPES node it - // is handed. This port's evalStatement has no such blanket check - // (evalChildren owns it), so the equivalent stop is made explicit - // here. - checkDebug(child, ctx); - evalStatement(child, ctx); + // A single-element evalChildren() call, not a hand-rolled + // checkDebug+evalStatement pair -- gives the wrapped child its own + // statement-level stop exactly like the old explicit checkDebug() + // did (evalChildren's own per-statement loop fires the identical + // check, at the identical point, for a lone non-declaration + // non-ModularLet node), while ALSO trying the compiled + // Op::CallModule path first (tryRunCompiledChildren, the + // NativeStatement-gap fix) -- so `#recur(n-1);` gets the same + // depth-safe treatment translate()/union()/let()/intersection_for + // already do, instead of always falling through evalStatement -> + // evalModularCall -> evalUserModule capped at kMaxUserCallDepth=30. + // No double stop: whichever path runs (compiled Op::CallModule, + // compiled Op::NativeStatement, or the native loop) fires the + // check exactly once, same as evalChildren's own existing callers. + evalChildren(std::vector{&child}, ctx); return CSGParams{}; }); } diff --git a/src/user_calls.cpp b/src/user_calls.cpp index 5a67b05..d6f4dbf 100644 --- a/src/user_calls.cpp +++ b/src/user_calls.cpp @@ -143,6 +143,28 @@ void Evaluator::releaseVmFrame(std::unique_ptr frame) { frame->stack.clear(); frame->slots.clear(); frame->bound.clear(); + // A REUSED (pooled) frame does NOT get VmFrame's own default member + // initializers re-applied -- those only fire for a genuinely fresh + // std::make_unique() (acquireVmFrame's own empty-pool + // fallback). Only pushBracketedModuleFrame ever sets ownsModuleSplice + // true (and moduleRandsBefore/moduleSpliceCallNode alongside it); no + // OTHER push site (pushBareFrame, pushBracketedCallFrame, + // runCompiledFunction(FromBound), a parameter default's own frame) + // ever resets it back to false. Without this, a frame released here + // after owning a module's own splice, then reacquired later for an + // ordinary FUNCTION call, keeps ownsModuleSplice=true -- if THAT + // function call is later torn down by an exception (teardownVmCall + // StackDownTo), its own `if (frame->ownsModuleSplice) treeStack_. + // pop_back();` fires for a frame that never pushed a treeStack_ entry + // of its own, silently popping one too many and corrupting treeStack_ + // for everything after. Caught for real via a heap-buffer-overflow + // (ASan) inside teardownVmCallStackDownTo's own treeStack_.pop_back(), + // triggered by BOSL2's poly_roots() -- a non-tail self-recursive + // function, reusing a frame previously released by an earlier, + // unrelated MODULE call -- throwing partway through. + frame->ownsModuleSplice = false; + frame->moduleRandsBefore = 0; + frame->moduleSpliceCallNode = nullptr; vmFramePool_.push_back(std::move(frame)); } diff --git a/tests/test_bytecode_compiler.cpp b/tests/test_bytecode_compiler.cpp index af19d25..d2bcb2f 100644 --- a/tests/test_bytecode_compiler.cpp +++ b/tests/test_bytecode_compiler.cpp @@ -1351,6 +1351,109 @@ TEST(ModuleBodyCompiles, DeepPureVmRecursionWithNonRecursingBuiltinWrappedLeafSu ASSERT_EQ(e.bodies.size(), 101u); } +// evalModifier used to dispatch its single wrapped child via a hand-rolled +// checkDebug()+evalStatement() pair, bypassing evalChildren entirely -- so +// a `#`/`!`-wrapped recursive module call never got the NativeStatement-gap +// fix's Op::CallModule treatment, unlike an otherwise-identical `translate`- +// wrapped call. Same shape as +// DeepPureVmRecursionWithNonRecursingBuiltinWrappedLeafSucceeds, just with +// `#` instead of `translate` wrapping the non-recursing leaf, proving the +// fix now applies here too. +TEST(ModuleBodyCompiles, DeepPureVmRecursionWithNonRecursingModifierWrappedLeafSucceeds) { + ScopedVm vm(true); + Evaluated e = evalSrc("module recur(n) {" + " if (n > 0) {" + " #translate([0,0,n]) cube(1);" + " recur(n - 1);" + " } else {" + " cube(1);" + " }" + "}" + "recur(100);"); + ASSERT_EQ(e.bodies.size(), 101u); +} + +// -- Leaf-statement compilation (Assignment/ModularEcho/ModularAssert/ ----- +// -- ModularLet get real bytecode instead of Op::NativeStatement) --------- + +TEST(ModuleBodyCompiles, AssignmentStatementCompilesWithDollarVarAndOverwrittenWarning) { + ScopedVm vm(true); + std::string captured = runCapturingEcho("$fn = 6;\n" + "function f() = $fn;\n" + "echo(f());\n" + "x = 1;\n" + "x = 2;\n" // overwritten warning + "echo(x);\n"); + EXPECT_NE(captured.find("ECHO: 6"), std::string::npos); + EXPECT_NE(captured.find("was assigned on line"), std::string::npos); + EXPECT_NE(captured.find("but was overwritten"), std::string::npos); + EXPECT_NE(captured.find("ECHO: 2"), std::string::npos); +} + +TEST(ModuleBodyCompiles, EchoStatementCompilesWithNamedAndPositionalArgs) { + ScopedVm vm(true); + EXPECT_EQ(runCapturingEcho("echo(1, b=2, 3);"), "ECHO: 1, b = 2, 3"); +} + +TEST(ModuleBodyCompiles, AssertStatementCompilesWithNamedArgsMessageAndEagerSideEffects) { + ScopedVm vm(true); + // Named args in reverse order (message before condition), PLUS a 3rd, + // logically-unused positional argument -- must still be evaluated + // eagerly for its own side effect (mirrors evalAssertStatement's own + // resolveArgs() call, unlike AssertOp's lazy message). + std::string captured = + runCapturingEcho("assert(message=\"unused, condition true\", condition=true, echo(\"side effect\"));\n" + "echo(\"reached\");\n"); + EXPECT_NE(captured.find("ECHO: \"side effect\""), std::string::npos); + EXPECT_NE(captured.find("ECHO: \"reached\""), std::string::npos); +} + +TEST(ModuleBodyCompiles, AssertStatementCompiledFormFailsWithNamedMessage) { + ScopedVm vm(true); + try { + evalSrc("assert(condition=false, message=\"custom\");"); + FAIL() << "expected EvalError"; + } catch (const EvalError& e) { + const std::string what = e.what(); + EXPECT_NE(what.find("Assertion 'false' failed"), std::string::npos) << what; + EXPECT_NE(what.find("\"custom\""), std::string::npos) << what; + } +} + +TEST(ModuleBodyCompiles, LetBlockStatementDoesNotSeeItsOwnEarlierSiblingAssignment) { + ScopedVm vm(true); + // The statement form's own documented divergence from let-EXPRESSION + // sequential visibility: b's RHS must see the OUTER a (1), not this + // same let-block's own a=2 -- b should be 2, not 3. + EXPECT_EQ(runCapturingEcho("a = 1;\n" + "let (a = 2, b = a + 1) { echo(a, b); }\n" + "echo(a);\n"), + "ECHO: 2, 2\nECHO: 1"); +} + +TEST(ModuleBodyCompiles, LetBlockStatementIsolatesDollarVarOverrideFromLaterStatements) { + ScopedVm vm(true); + EXPECT_EQ(runCapturingEcho("function f() = $fn;\n" + "$fn = 10;\n" + "let ($fn = 99) { echo(f()); }\n" + "echo(f());\n"), + "ECHO: 99\nECHO: 10"); +} + +TEST(ModuleBodyCompiles, NestedLetExpressionInsideCompiledStatementArgumentAllocatesSlotsCorrectly) { + // Exercises the numSlots/frame->slots fix -- a compiled module-body + // statement's own inline argument can itself contain a LetOp + // EXPRESSION (as opposed to the ModularLet STATEMENT form, above), + // which DOES use real local slots (unlike anything else in a module + // chunk) -- without frame->slots sized to chunk.numSlots, this + // indexes out of bounds. + ScopedVm vm(true); + EXPECT_EQ(runCapturingEcho("echo(let(x = 5, y = x + 1) x + y);\n" + "v = let(q = 3) q * q;\n" + "echo(v);\n"), + "ECHO: 11\nECHO: 9"); +} + TEST(ModuleBodyCompiles, RecursiveModuleThrowingPartwayUnwindsCleanlyAndVmFrameRunsAgain) { ScopedVm vm(true); Evaluator ev; From 04b1a4e2acf839c7fa7340d7b80a6403fa885078 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 15:52:34 -0700 Subject: [PATCH 2/3] Lower kMaxDriveVmNativeDepth 100->50: 100 segfaulted on Windows CI Windows CI's own crash-safety regression test (ModuleBodyCompiles.NativeReentrantRecursionHitsAControlledErrorInsteadOfCrashing) segfaulted at 100 -- confirms this native-reentry chain has a heavier real per-level frame cost than kMaxUserCallDepth's own (which needed the identical kind of comedown, an initial 50 down to 30, for a shallower chain). Set to exactly the locally-determined functional minimum (50, the smallest value that fixes the 3 real BOSL2 scripts this guard was regressing) with no headroom this time, since headroom is what crashed. Re-verified locally: full suite green both VM states, guard-safety test passes, and all 3 BOSL2 scripts still succeed at 50. Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/evaluator.hpp | 26 ++++++++++++-------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index af0999a..3298de0 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -1344,17 +1344,23 @@ class Evaluator { // mask -- all built on deeply layered _translate()/_show_highlight()/ // attachment-wrapper composition, not runaway recursion) that used to // render successfully but started failing with "Recursion too deep - // (native call stack)" once this guard shipped at 30. Empirically - // raised to the smallest value (binary search, local rebuild+retest) - // that lets all 3 succeed again (50), then doubled for headroom - // against future library growth -- NOT independently re-verified - // against Windows CI's own tighter stack margin the way - // kMaxUserCallDepth's own 30 was (that number came DOWN from an - // initial 50 specifically because 50 segfaulted there) -- if CI ever - // flags this as unsafe on a real run, come back down, don't guess - // again locally. Deliberately its own named constant (not a reuse of + // (native call stack)" once this guard shipped at 30. + // + // Locally binary-searched (macOS) to 50, the smallest value that lets + // all 3 succeed again, then FIRST tried doubled to 100 for headroom -- + // that immediately segfaulted on Windows CI + // (ModuleBodyCompiles.NativeReentrantRecursionHitsAControlledError + // InsteadOfCrashing, its own guard-safety regression test), confirming + // this chain's real per-level native-frame cost is heavier than + // kMaxUserCallDepth's own (which needed the SAME kind of comedown, an + // initial 50 to 30, for a shallower chain). Set to exactly the + // locally-determined functional minimum (50), no headroom this time -- + // headroom is what crashed. If a future CI run ever segfaults here + // again, come back down further via that same signal; don't rely on + // local (macOS/Linux) testing alone to judge Windows stack margin. + // Deliberately its own named constant (not a reuse of // kMaxUserCallDepth) so the two can be recalibrated independently. - static constexpr size_t kMaxDriveVmNativeDepth = 100; + static constexpr size_t kMaxDriveVmNativeDepth = 50; // Bracketed public in place: Op::CallModule's own runtime handler and // driveVm's module-frame completion branch (bytecode_vm.cpp, a From a31b230d3c9ca7325d062c1d8458ab7cb90a38e0 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 31 Jul 2026 16:04:33 -0700 Subject: [PATCH 3/3] Lower kMaxDriveVmNativeDepth 50->40: 50 also segfaulted on Windows CI Bisecting toward Windows' real safe ceiling for this native-reentry chain: 30 is known-safe (PR #60's own successful merge), 50 is now known-unsafe (segfaulted on the guard-safety regression test both at 100 and 50). Trying 40, the midpoint -- still functionally sufficient (all 3 real BOSL2 scripts this guard was regressing still render locally at this value), full suite green locally both VM states. Awaiting CI to confirm Windows safety before considering this settled. Co-Authored-By: Claude Sonnet 5 --- include/openscad_cpp_evaluator/evaluator.hpp | 34 ++++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 3298de0..e630b68 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -1346,21 +1346,27 @@ class Evaluator { // render successfully but started failing with "Recursion too deep // (native call stack)" once this guard shipped at 30. // - // Locally binary-searched (macOS) to 50, the smallest value that lets - // all 3 succeed again, then FIRST tried doubled to 100 for headroom -- - // that immediately segfaulted on Windows CI - // (ModuleBodyCompiles.NativeReentrantRecursionHitsAControlledError - // InsteadOfCrashing, its own guard-safety regression test), confirming - // this chain's real per-level native-frame cost is heavier than - // kMaxUserCallDepth's own (which needed the SAME kind of comedown, an - // initial 50 to 30, for a shallower chain). Set to exactly the - // locally-determined functional minimum (50), no headroom this time -- - // headroom is what crashed. If a future CI run ever segfaults here - // again, come back down further via that same signal; don't rely on - // local (macOS/Linux) testing alone to judge Windows stack margin. - // Deliberately its own named constant (not a reuse of + // Locally binary-searched (macOS) to a first candidate of 50, the + // smallest value tried that let all 3 succeed again, then FIRST tried + // doubled to 100 for headroom -- that immediately segfaulted on + // Windows CI (ModuleBodyCompiles. + // NativeReentrantRecursionHitsAControlledErrorInsteadOfCrashing, its + // own guard-safety regression test). Backed off to 50 -- STILL + // segfaulted there. Confirms this chain's real per-level native-frame + // cost is heavier than kMaxUserCallDepth's own (which needed the SAME + // kind of comedown, an initial 50 down to 30, for a shallower chain), + // and that Windows' real safe ceiling for THIS chain sits somewhere + // below 50, above the already-Windows-verified-safe 30 (confirmed + // safe by PR #60's own successful merge, before this value ever + // changed). Currently at 40 -- ALSO still functionally sufficient + // (all 3 real scripts still render locally at this value) -- bisecting + // downward toward Windows' actual ceiling via successive CI runs, one + // step at a time; if 40 itself is still unsafe there, keep coming + // down. Do not raise this value again based on local (macOS/Linux) + // testing alone -- only a real Windows CI pass is evidence of safety + // here. Deliberately its own named constant (not a reuse of // kMaxUserCallDepth) so the two can be recalibrated independently. - static constexpr size_t kMaxDriveVmNativeDepth = 50; + static constexpr size_t kMaxDriveVmNativeDepth = 40; // Bracketed public in place: Op::CallModule's own runtime handler and // driveVm's module-frame completion branch (bytecode_vm.cpp, a