From b89b736cf08fd3cf84c86cfa31a95ff941ee96fe Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 8 Aug 2026 13:51:36 -0700 Subject: [PATCH] Aim step-into-child at the children a children() call forwards Stepping to a child from inside a module's `children();` ran the script to completion instead of stopping at the child. childStatementPositions returned a node's own children, and a `children()` call has none -- it forwards the enclosing invocation's, which live on the context, not on the node. With no target to match, the step degraded into a continue: the one statement whose entire job is to run the children was the one place a debugger could not step into them. It now reads ctx.childrenNodes for that case. Every forwarded child is returned even when an index argument (children(0), children([1:2])) will run only some -- a position that is never reached can never be stopped at, so the extra targets are inert. Found while chasing a report that step-into-child failed for a built-in child. The built-in was a red herring: cube() as a forwarded child steps fine now, and a user module failed identically before. What mattered was where the pause was, not what the child happened to be. Co-Authored-By: Claude Opus 5 (1M context) --- pyproject.toml | 2 +- src/debug_profile.cpp | 26 +++++++++++++++++++++++-- tests/test_debug_hooks.cpp | 40 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7a4f8f1..804b924 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.26.0" +version = "0.26.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/debug_profile.cpp b/src/debug_profile.cpp index 11b2278..ebf8624 100644 --- a/src/debug_profile.cpp +++ b/src/debug_profile.cpp @@ -9,9 +9,31 @@ namespace { // call), if any. See Evaluator::lastChildrenPositions()'s own doc comment // for what this feeds. Mirrors the reference's own // Evaluator._child_statement_positions. -std::optional>> childStatementPositions(const oscad::ASTNode& node) { +// Where a debugger's "step into child" should aim from `node`. +// +// Normally the node's own children -- `framed(20) leaf(3);` aims at +// `leaf(3)`. But a `children()` call has no children of its own: it +// forwards the enclosing invocation's, which live on the context. Without +// that case, stepping to a child from the one statement whose entire job +// is to run the children found nothing to aim at, and the step silently +// degraded into a continue -- running to the next breakpoint or off the +// end of the script. +std::optional>> childStatementPositions(const oscad::ASTNode& node, + const EvalContext& ctx) { if (node.kind() != oscad::NodeKind::ModularCall) return std::nullopt; const auto& call = static_cast(node); + if (call.name && call.name->name == "children") { + if (!ctx.childrenNodes || ctx.childrenNodes->empty()) return std::nullopt; + std::vector> forwarded; + for (const oscad::ASTNode* c : *ctx.childrenNodes) { + if (c) forwarded.emplace_back(c->position().origin, c->position().line); + } + if (forwarded.empty()) return std::nullopt; + // Every forwarded child, even when an index argument (children(0), + // children([1:2])) will run only some: a position that is never + // reached can never be stopped at, so the extra targets are inert. + return forwarded; + } std::vector> positions; for (const auto& c : call.children) { if (c->kind() == oscad::NodeKind::Assignment || c->kind() == oscad::NodeKind::ModuleDeclaration || @@ -121,7 +143,7 @@ void Evaluator::checkDebug(const oscad::ASTNode& node, EvalContext& ctx, bool fo } const int depth = static_cast(callStack_.size()); const DebugFramesFn getFrame = [this, &ctx]() { return buildDebugFrames(&ctx); }; - lastChildrenPositions_ = childStatementPositions(node); + lastChildrenPositions_ = childStatementPositions(node, ctx); DebugAction action = debugHooks_.debugHook(pos.line, depth, forced, exprLevel, pos.origin, callStack_, getFrame); for (auto& [k, v] : action.mods) ctx.let_->set(k, v); diff --git a/tests/test_debug_hooks.cpp b/tests/test_debug_hooks.cpp index 88a121c..0b5a6a5 100644 --- a/tests/test_debug_hooks.cpp +++ b/tests/test_debug_hooks.cpp @@ -641,6 +641,46 @@ TEST(DebugHooksParity, CallSiteOnItsOwnLineStillStops) { EXPECT_EQ(line4, 2); } +// Step-into-child aims at the children a `children()` call forwards, not +// at its own (it has none). Without this, the one statement whose entire +// job is to run the children offered a debugger nothing to aim at, and the +// step degraded into a continue -- running off the end of the script. +TEST(DebugHooksChildren, ChildrenCallForwardsItsCallersChildrenAsStepTargets) { + // Line 5 is the child; line 2 is the `children();` that forwards it. + const char* src = "module framed(gap) {\n" + " children();\n" + "}\n" + "framed(20)\n" + " leaf();\n" + "module leaf() { cube(1); }\n"; + std::vector>>> seen; + DebugHooks hooks; + Evaluator* evp = nullptr; + hooks.debugHook = [&](int line, int, bool, bool exprLevel, const std::string&, + const std::vector&, const DebugFramesFn&) { + if (!exprLevel && evp && evp->lastChildrenPositions()) + seen.emplace_back(line, *evp->lastChildrenPositions()); + return DebugAction{}; + }; + Evaluator ev(EchoFn{}, nullptr, nullptr, hooks); + evp = &ev; + auto ast = parseSrc(src); + auto scope = oscad::buildScopes(ast); + EvalContext ctx = EvalContext::makeRoot(scope.get()); + ev.evaluate(ast, ctx); + + // At `children();` on line 2, the target is the forwarded child on + // line 5 -- not nothing, and not the children() call's own position. + bool found = false; + for (const auto& [line, positions] : seen) { + if (line != 2) continue; + found = true; + ASSERT_EQ(positions.size(), 1u); + EXPECT_EQ(positions.front().second, 5); + } + EXPECT_TRUE(found) << "no statement-level stop at the children() call"; +} + TEST(DebugHooksParity, BuiltinFunctionCallGetsNoCallSiteStop) { // Only the assignment is statement-level; the 3 expr-level stops are // the argument list literal's own elements, not a call-site stop.