diff --git a/CLAUDE.md b/CLAUDE.md index 8da524c..b243d31 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -484,7 +484,10 @@ grep for `ponytail:`. and `builtinChildren` (children()/children(N), deferred evaluation; `children(separate=true)` additionally marks each forwarded node `separateOperand` so the enclosing union/difference/intersection treats them as separate operands rather than one grouped one -- - a BelfrySCAD extension, see `Evaluator::appendGroupSizes`). `evalFunctionCall`'s + a BelfrySCAD extension, see `Evaluator::appendGroupSizes`. The marks stop at a user-module + boundary: only the children() call's own splice honours them + (`spliceModuleChildren`'s `honorSeparateMarks`), so a module wrapping such a call wraps its + children as usual and its own caller sees one operand). `evalFunctionCall`'s precedence order (checked in this exact sequence): `import` (special-cased, its own module/expression-context split) → `isBuiltinFunctionName` (function_builtins.hpp — always wins if true) → user function lookup → function-literal *value* probe → "unknown function" warning. diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 7c05765..01f673b 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -999,8 +999,13 @@ class Evaluator { // (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). + // `honorSeparateMarks` is true only for a children() call's OWN splice. + // A user module's splice passes false, so it wraps its marked children + // as usual and the marks stop there -- children(separate=true) applies + // to the operator enclosing that call, not to whatever encloses the + // module the call happens to sit in. void spliceModuleChildren(std::vector> children, std::uint64_t randsBefore, - const oscad::ASTNode& callNode); + const oscad::ASTNode& callNode, bool honorSeparateMarks = false); private: Value evalUserFunction(const std::string& name, const oscad::FunctionDeclaration& decl, const std::vector>& arguments, EvalContext& ctx, diff --git a/pyproject.toml b/pyproject.toml index 912e6dd..87e0ac8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.42.0" +version = "0.42.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/bytecode_vm.cpp b/src/bytecode_vm.cpp index b92e4d9..e23e4fe 100644 --- a/src/bytecode_vm.cpp +++ b/src/bytecode_vm.cpp @@ -516,8 +516,12 @@ Value driveVm(Evaluator& ev, size_t floor) { if (isModule && ownsModuleSplice) { std::vector> children = std::move(ev.treeStack_.back()); ev.treeStack_.pop_back(); + // separateChildren is set only by pushChildrenForwardFrame, + // so it also answers "is this a children() splice?" -- a + // user module's frame always passes false and wraps. if (separateChildren) markSeparateOperands(children, 0); - ev.spliceModuleChildren(std::move(children), moduleRandsBefore, *moduleSpliceCallNode); + ev.spliceModuleChildren(std::move(children), moduleRandsBefore, *moduleSpliceCallNode, + /*honorSeparateMarks=*/separateChildren); } if (isFloorFrame) { finalResult = std::move(result); // unused by a module caller (runCompiledModuleBody ignores it) @@ -1076,7 +1080,8 @@ Value driveVm(Evaluator& ev, size_t floor) { std::vector> children = std::move(ev.treeStack_.back()); ev.treeStack_.pop_back(); if (separate) markSeparateOperands(children, 0); - ev.spliceModuleChildren(std::move(children), randsBefore, *callNode); + ev.spliceModuleChildren(std::move(children), randsBefore, *callNode, + /*honorSeparateMarks=*/true); ++f.pc; } break; diff --git a/src/csg_resolve.cpp b/src/csg_resolve.cpp index f3e73c4..fa2fac3 100644 --- a/src/csg_resolve.cpp +++ b/src/csg_resolve.cpp @@ -126,9 +126,10 @@ void Evaluator::evalModularCall(const oscad::ModularCall& node, EvalContext& ctx // under a synthetic (display-only, is_builtin=false) "union" node so // the tree still reads as one shape at this call site. Mirrors // _eval_statement's splice branch exactly. - const bool splice = (name == "children" && isBuiltin) || !isBuiltin; + const bool isChildrenCall = (name == "children" && isBuiltin); + const bool splice = isChildrenCall || !isBuiltin; if (splice) { - spliceModuleChildren(std::move(children), randsBefore, node); + spliceModuleChildren(std::move(children), randsBefore, node, isChildrenCall); return; } @@ -147,7 +148,7 @@ void Evaluator::evalModularCall(const oscad::ModularCall& node, EvalContext& ctx } void Evaluator::spliceModuleChildren(std::vector> children, std::uint64_t randsBefore, - const oscad::ASTNode& callNode) { + const oscad::ASTNode& callNode, bool honorSeparateMarks) { 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 @@ -161,7 +162,18 @@ void Evaluator::spliceModuleChildren(std::vector> child // children(separate=true) marked these to start their own operand // groups, and the group walk only inspects the enclosing frame's top // level -- so the wrapper would hide the marks. Splice instead. + // + // Only for the children() call's own splice. A USER MODULE wrapping + // such a call wraps as usual, which hides the marks and is exactly what + // makes separateness stop at the module boundary: + // + // module pass() { children(separate=true); } + // difference() pass() { a; b; } // a | b, not a - b + // + // pass() is its own shape; that it happens to forward its children + // separately is its business, not its caller's. const bool anySeparate = + honorSeparateMarks && std::any_of(children.begin(), children.end(), [](const auto& c) { return c->separateOperand; }); if (children.size() > 1 && !anySeparate) { auto unionNode = std::make_unique(); diff --git a/tests/test_children_separate.cpp b/tests/test_children_separate.cpp index 939d20f..1e6d1b6 100644 --- a/tests/test_children_separate.cpp +++ b/tests/test_children_separate.cpp @@ -179,16 +179,34 @@ TEST(ChildrenSeparate, DoesNotAffectALaterUnrelatedDifference) { } } -TEST(ChildrenSeparate, PropagatesThroughAUserModuleWrapper) { - // A judgement call, pinned so it cannot drift silently: the mark rides - // on the nodes, so a module whose whole body is children(separate=true) - // passes the separateness outward to whatever encloses the call. - // Retreating to "stop at the module boundary" is any_of -> all_of in - // spliceModuleChildren. +TEST(ChildrenSeparate, StopsAtAUserModuleBoundary) { + // separate=true applies to the operator enclosing THAT children() call, + // not to whatever encloses the module the call sits in. pass() is its + // own shape; that it forwards its children separately is its business, + // and difference() outside it still sees one operand. + // + // Mechanically: a user module's splice wraps its children as usual, + // which hides the marks from the group walk. Only the children() call's + // own splice honours them (spliceModuleChildren's honorSeparateMarks). for (bool vm : {false, true}) { ScopedVm guard(vm); Evaluated e = evalSrc(std::string("module pass() { children(separate=true); }\n" "difference() pass() ") + kThreeChildren); + EXPECT_NEAR(totalVolume(e.bodies), kCube, 1e-6) << "vm=" << vm; + } +} + +TEST(ChildrenSeparate, StillWorksInsideTheModuleThatAsksForIt) { + // The other half of the boundary rule: a module that puts the operator + // and the children(separate=true) call together still works, however + // deeply that module is itself nested. + for (bool vm : {false, true}) { + ScopedVm guard(vm); + Evaluated e = evalSrc(std::string("module frame() { difference() children(separate=true); }\n" + "module outer() { frame() { cube(50, center=true);" + " translate([-15,0,0]) cube(10, center=true);" + " translate([ 15,0,0]) cube(10, center=true); } }\n" + "outer();")); EXPECT_NEAR(totalVolume(e.bodies), kCube - 2 * kSmall, 1e-6) << "vm=" << vm; } }