Collapse the call-site debug stop that duplicates its own statement

include/openscad_cpp_evaluator/evaluator.hpp

@@ -425,6 +425,13 @@ class Evaluator {
         fastContinueHookSkippable_ = hookSkippable;
     }
 
+    // The statement checkpoint currently in progress at each call depth,
+    // for the call-site collapse described on checkDebug. Per depth, not a
+    // single slot: `a = [f(1), f(2), f(3)];` runs each callee's own body
+    // checkpoints in between, and those must not be mistaken for the
+    // caller having moved on to a new statement.
+    std::vector<std::pair<int, std::string>> lastStmtByDepth_;
+
     // The other half of hook-skippable mode's safety net. checkDebug()'s
     // whole premise (see its own doc comment, debug_profile.cpp) is that it
     // can skip calling into Python for a line with no breakpoint -- but the
@@ -475,7 +482,18 @@ class Evaluator {
     // (function body entry), and builtins/control.cpp's
     // resolveBreakpoint/resolveIntersectionFor (free functions). Mirrors
     // Evaluator._check_debug, including its parameter defaults.
-    void checkDebug(const oscad::ASTNode& node, EvalContext& ctx, bool forced = false, bool exprLevel = false);
+    // `callSite` marks the stop fired just before descending into a user
+    // function or function literal. It is a real, steppable stop -- a
+    // debugger's Step Over should pause on the call line -- but it is NOT
+    // a second execution of the line it sits on. When it lands on the same
+    // line and depth as the statement checkpoint immediately before it (as
+    // in `x = f(y);`, where the assignment and the call are one line), it
+    // is dropped, so a breakpoint there fires once per execution instead
+    // of twice. A call on its own line, or on a different line from the
+    // enclosing statement, still stops -- which is what keeps stepping
+    // through a list comprehension alternating for-line/call-line.
+    void checkDebug(const oscad::ASTNode& node, EvalContext& ctx, bool forced = false, bool exprLevel = false,
+                    bool callSite = false);
 
     // (origin, line) for each top-level, non-declaration child of the
     // node checkDebug() was just called with -- i.e., if that node is a

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build"
 
 [project]
 name = "openscad_cpp_evaluator"
-version = "0.25.0"
+version = "0.26.0"
 description = "C++ OpenSCAD evaluator with Python bindings"
 readme = "README.md"
 requires-python = ">=3.12"

src/debug_profile.cpp

@@ -74,9 +74,21 @@ std::vector<DebugFrame> Evaluator::buildDebugFrames(const EvalContext* ctx) cons
     return frames;
 }
 
-void Evaluator::checkDebug(const oscad::ASTNode& node, EvalContext& ctx, bool forced, bool exprLevel) {
+void Evaluator::checkDebug(const oscad::ASTNode& node, EvalContext& ctx, bool forced, bool exprLevel,
+                            bool callSite) {
     if (!debugHooks_.debugHook) return;
     const oscad::Position& pos = node.position();
+    const size_t stmtDepth = callStack_.size();
+    if (lastStmtByDepth_.size() <= stmtDepth) lastStmtByDepth_.resize(stmtDepth + 1, {-1, std::string{}});
+    if (callSite && !forced && lastStmtByDepth_[stmtDepth] == std::make_pair(pos.line, pos.origin)) {
+        // The statement this call belongs to already stopped on this exact
+        // line, at this depth. See checkDebug's declaration for why this
+        // one is dropped.
+        return;
+    }
+    if (!exprLevel && !callSite) {
+        lastStmtByDepth_[stmtDepth] = {pos.line, pos.origin};
+    }
     // Fast-continue's hook-skippable mode (setFastContinueBreakpoints' own
     // doc comment): a plain "Continue" with no step pending needs the debug
     // hook called ONLY for a line that actually has a breakpoint -- every

src/user_calls.cpp

@@ -547,7 +547,11 @@ std::variant<Value, Evaluator::TailStep, Evaluator::NotTailStep> Evaluator::simp
                 const oscad::ASTNode* declNode = ctx.scope->lookupFunction(leftId->name);
                 if (declNode && declNode->kind() == oscad::NodeKind::FunctionDeclaration) {
                     const auto& decl = static_cast<const oscad::FunctionDeclaration&>(*declNode);
-                    checkDebug(n, ctx); // call-site stop, per hop (see evalFunctionCall's)
+                    // Expression-level: a call sitting inside a larger
+                    // statement is not its own statement, and pausing here
+                    // as well as at the statement makes a breakpoint on
+                    // that line fire twice per execution.
+                    checkDebug(n, ctx, /*forced=*/false, /*exprLevel=*/false, /*callSite=*/true); // call-site stop, per hop
                     const bool hasChunk = useBytecodeVm() && lookupOrCompileChunk(decl) != nullptr;
                     std::optional<TailStep> step = tryTailStepFor(leftId->name, decl, decl.parameters, *decl.expr,
                                                                    hasChunk, n.arguments, ctx, n.position());
@@ -570,7 +574,7 @@ std::variant<Value, Evaluator::TailStep, Evaluator::NotTailStep> Evaluator::simp
             if (const auto* closurePtr = std::get_if<ClosurePtr>(&funcVal); closurePtr && *closurePtr) {
                 const Closure& closure = **closurePtr;
                 const oscad::FunctionLiteral& funcNode = *closure.node;
-                checkDebug(n, ctx); // call-site stop, function-literal callee
+                checkDebug(n, ctx, /*forced=*/false, /*exprLevel=*/false, /*callSite=*/true); // call-site stop, function-literal callee
                 const bool hasChunk = useBytecodeVm() && lookupCompiledLiteralChunk(funcNode) != nullptr;
                 std::optional<TailStep> step =
                     tryTailStepFor("<function literal>", funcNode, funcNode.parameters, *funcNode.body, hasChunk,
@@ -886,7 +890,7 @@ Value Evaluator::evalFunctionCall(const oscad::PrimaryCall& node, EvalContext& c
             // evalUserFunctionCore). Builtins deliberately get none --
             // mirrors _eval_function_call, where only the user-function and
             // function-literal branches call _check_debug.
-            checkDebug(node, ctx);
+            checkDebug(node, ctx, /*forced=*/false, /*exprLevel=*/false, /*callSite=*/true);
             return evalUserFunction(leftId->name, static_cast<const oscad::FunctionDeclaration&>(*decl), node.arguments,
                                      ctx, &node);
         }
@@ -899,7 +903,7 @@ Value Evaluator::evalFunctionCall(const oscad::PrimaryCall& node, EvalContext& c
     // unknown callee gets exactly one warning below, not two.
     Value funcVal = leftId ? evalIdentifier(leftId->name, &leftId->position(), ctx, false) : evalExpr(*node.left, ctx);
     if (const auto* closurePtr = std::get_if<ClosurePtr>(&funcVal); closurePtr && *closurePtr) {
-        checkDebug(node, ctx); // same call-site stop, function-literal callee
+        checkDebug(node, ctx, /*forced=*/false, /*exprLevel=*/false, /*callSite=*/true); // same call-site stop, function-literal callee
         return evalFunctionLiteral(**closurePtr, node.arguments, ctx, &node);
     }
 

tests/test_bytecode_compiler.cpp

@@ -197,13 +197,13 @@ TEST(BytecodeCompiler, DollarPrefixedParameterCompiles) {
     EXPECT_EQ(runCapturingEcho("function withFn($fn) = $fn;\necho(let($fn = 99) withFn());"), "ECHO: undef");
     // The proof this is actually running compiled, not silently falling
     // back: same ternary-bodied shape and technique as
-    // FastContinueWithNoBreakpointInFunctionUsesVm above (3 stops =
-    // compiled, 5 = interpreted), with $fn substituted in for the plain
+    // FastContinueWithNoBreakpointInFunctionUsesVm above (2 stops =
+    // compiled, 4 = interpreted), with $fn substituted in for the plain
     // parameter both as the declared name and every body reference.
     const int stops = countDebugHookStops("function f($fn) = $fn > 0 ? $fn + 1 : $fn - 1;\n"
                                            "echo(f(5));",
                                            std::unordered_map<std::string, std::set<int>>{{"<string>", {2}}});
-    EXPECT_EQ(stops, 3);
+    EXPECT_EQ(stops, 2);
 }
 
 TEST(BytecodeCompiler, UndeclaredDollarNamedArgumentReachesDynInsideCompiledFunction) {
@@ -828,7 +828,7 @@ TEST(BytecodeCompiler, ClosureWithDollarParameterNowCompilesToo) {
     // silent even for a closure capturing an enclosing binding with its
     // own $-parameter.
     const int stops = countDebugHookStops(script, std::unordered_map<std::string, std::set<int>>{{"<string>", {2}}});
-    EXPECT_EQ(stops, 3);
+    EXPECT_EQ(stops, 2);
 }
 
 // -- Tail-call optimization, VM path (Phase B) -----------------------------
@@ -1156,7 +1156,7 @@ TEST(BytecodeCompiler, DebugAttachedWithoutFastContinueAlwaysInterprets) {
     const int stops = countDebugHookStops("function f(x) = x > 0 ? x + 1 : x - 1;\n"
                                            "echo(f(5));",
                                            std::nullopt);
-    EXPECT_EQ(stops, 5);
+    EXPECT_EQ(stops, 4);
 }
 
 TEST(BytecodeCompiler, FastContinueWithNoBreakpointInFunctionUsesVm) {
@@ -1167,7 +1167,7 @@ TEST(BytecodeCompiler, FastContinueWithNoBreakpointInFunctionUsesVm) {
     const int stops = countDebugHookStops("function f(x) = x > 0 ? x + 1 : x - 1;\n"
                                            "echo(f(5));",
                                            std::unordered_map<std::string, std::set<int>>{{"<string>", {2}}});
-    EXPECT_EQ(stops, 3);
+    EXPECT_EQ(stops, 2);
 }
 
 TEST(BytecodeCompiler, FastContinueWithBreakpointInsideFunctionStillInterprets) {

tests/test_debug_hooks.cpp

@@ -595,17 +595,50 @@ TEST(DebugHooksParity, ListCompCForFiresEveryInitAndIncrSeparately) {
     EXPECT_EQ(stops.size(), 14u);           // + 3 condition checks + 2 body expressions
 }
 
-// (13) User function / function-literal call sites: a statement-level stop
-// in the CALLER's context, before the callee's own body-entry stop.
+// (13) User function / function-literal call sites: a stop in the CALLER's
+// context, before the callee's own body-entry stop.
+//
+// DELIBERATE DIVERGENCE from the reference, and only for the case that
+// duplicates: when the call sits on the same line and depth as the
+// statement that just stopped, its call-site stop is dropped. Otherwise
+// every consumer treating a statement-level stop as a breakpoint hit
+// pauses twice for one execution of `a = double(5);` -- BelfrySCAD's
+// debugger stuttered on Continue and reported the same loop iteration
+// twice. Consumers cannot collapse it themselves: fast-continue skips the
+// checkpoints in between, so a duplicate is indistinguishable from a
+// genuine second visit to the line.
+//
+// A call on a different line from its enclosing statement still stops --
+// see CallSiteOnItsOwnLineStillStops, which is what keeps Step Over
+// through a list comprehension alternating for-line/call-line.
 TEST(DebugHooksParity, UserFunctionCallSiteStopsBeforeDescendingIntoTheCallee) {
-    // line 2 twice (assignment, then call site), then line 1 (body entry).
+    // line 2 (assignment; the call site on the same line is collapsed),
+    // then line 1 (body entry).
     EXPECT_EQ(recordStops("function double(x) = x * 2;\na = double(5);\n"),
-              (std::vector<Stop>{{2, false, false}, {2, false, false}, {1, false, false}}));
+              (std::vector<Stop>{{2, false, false}, {1, false, false}}));
 }
 
 TEST(DebugHooksParity, FunctionLiteralCallSiteAlsoStops) {
     EXPECT_EQ(recordStops("f = function(x) x * 2;\na = f(5);\n"),
-              (std::vector<Stop>{{1, false, false}, {2, false, false}, {2, false, false}, {1, false, false}}));
+              (std::vector<Stop>{{1, false, false}, {2, false, false}, {1, false, false}}));
+}
+
+// The other half of the rule above: a call that is NOT on its enclosing
+// statement's line keeps its own stop, so stepping still pauses on the
+// call before descending. This is the list-comprehension shape.
+TEST(DebugHooksParity, CallSiteOnItsOwnLineStillStops) {
+    const std::vector<Stop> stops = recordStops("function fx(x) = x*2;\n"
+                                                 "a = [\n"
+                                                 "    for (i = [0:1])\n"
+                                                 "        fx(i)\n"
+                                                 "];\n");
+    // Line 4 holds only the call. Two iterations, so two call-site stops,
+    // both statement-level and none collapsed -- the statement that
+    // stopped before each was the `for` on line 3.
+    int line4 = 0;
+    for (const Stop& s : stmtStops(stops))
+        if (s.line == 4) ++line4;
+    EXPECT_EQ(line4, 2);
 }
 
 TEST(DebugHooksParity, BuiltinFunctionCallGetsNoCallSiteStop) {

tests/test_tail_calls.cpp

@@ -283,9 +283,12 @@ TEST(TailCalls, DebugHookFiresPerHopInsideATailChain) {
     // abandoning TCO whenever a debugger is attached.
     EXPECT_GT(calls, 1000);
     // All statement-level stops on line 1 (the whole function declaration):
-    // 501 ternary stops (n = 500..0), 500 recursive call-site stops, and the
-    // single body-entry stop.
-    EXPECT_EQ(bodyEntry, 501 + 500 + 1);
+    // 501 ternary stops (n = 500..0) and the single body-entry stop. The
+    // 500 recursive call-site stops still fire -- they are counted in
+    // `calls` above -- but as expression-level, since a call inside a
+    // larger expression is not a statement of its own (see
+    // DebugHooksParity.UserFunctionCallSiteStopsBeforeDescendingIntoTheCallee).
+    EXPECT_EQ(bodyEntry, 501 + 1);
 }
 
 TEST(TailCalls, DeepNonTailRecursionHitsAControlledErrorInsteadOfCrashingInterpreted) {