Extend Op::PushBuiltinWrap to hull/minkowski/render/extrude/offset/roof - #68
Conversation
Every remaining non-leaf builtin whose own children can wrap a recursive call still fell to Op::NativeStatement -- one real native C++ reentry per level, capped at kMaxDriveVmNativeDepth=40 -- exactly the class of risk Op::PushBuiltinWrap (translate/color/#/%/!) and Op::PushCsgWrap (union/difference/intersection) already closed for their own subsets. hull()/minkowski()/render()/linear_extrude()/rotate_extrude()/ projection()/offset()/roof() all share resolveTransform's own "resolve args, evaluate children, build params" shape (verified against each of their existing native resolve functions), so they reuse Op:: PushBuiltinWrap's own bracket via 6 new Kind values instead of a new opcode set: Passthrough (hull/minkowski/render -- empty params) and LinearExtrude/RotateExtrude/Projection/Offset compute their params at Push time, same as Transform/Color already do (verified side-effect- free); Roof computes its params at Pop time instead, since its "Unknown roof method" warning is an observable side effect that must stay ordered after any child's own echo/warn output, matching native resolveRoof's own evalChildren-then-compute-params order exactly. Each computeXParams helper is extracted from its resolve function's existing tail (shared by both the native and compiled path, same pattern as the pre-existing computeTransformParams/computeColorParams) -- no behavior change to the interpreter path. Only intersection_for remains uncovered: its own group_sizes are keyed by RUNTIME loop iteration count, not a static source-statement list the way union/difference/intersection's are, so it needs real loop- compilation machinery neither existing bracket shape provides -- a separate, larger follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Verification follow-up: while working on the intersection_for follow-up (#69), found that my local CLI binary ( The unit test suite itself was never affected (CMake always relinks Confirms the fix is real, not just unit-test-shaped. No code changes needed here — just flagging the gap in case CI's own from-scratch build didn't already make this moot (it should have). |
Summary
Op::NativeStatement— one real native C++ reentry per level, capped atkMaxDriveVmNativeDepth=40(same riskOp::PushBuiltinWrapClose the NativeStatement gap for translate/color/modifier-wrapped recursion #63 andOp::PushCsgWrapAdd Op::PushCsgWrap: eliminate union/difference/intersection's native reentry #67 already closed for their own subsets).hull()/minkowski()/render()/linear_extrude()/rotate_extrude()/projection()/offset()/roof()all shareresolveTransform's own "resolve args, evaluate children, build params" shape, so they reuseOp::PushBuiltinWrap's existing bracket via 6 newKindvalues instead of a new opcode set — no new mechanism, just widening the one that already exists.roof()'s "Unknown roof method" warning is an observable side effect, so its params are computed at Pop time (after children) instead of Push time like every other kind here, to preserve native's own echo/warn ordering exactly. Verified with a dedicated ordering test.intersection_forremains uncovered — itsgroup_sizesare keyed by runtime loop iteration count, not a static source-statement list, so it needs real loop-compilation machinery neither existing bracket shape provides. Flagged as a separate, larger follow-up (not silently skipped).Test plan
OSCAD_BYTECODE_VMon (default) and off, up from 727 (10 new tests: one recursion-depth test per builtin,$-arg propagation into a compiled wrap, and the roof warn-ordering test).offset(r=1)nested 1500 levels deep is a real, unbounded geometric operation (each level's rounded-join offset grows both size and vertex count) — hung for minutes before being rewritten tooffset(delta=0)(a real no-op at every level, same opcode path, none of the compounding cost).examples/fractal_tree.scadrenders to a byte-for-byte identical STL before/after this change.🤖 Generated with Claude Code