Switch the behavior of next and step on function calls in the debugger

Resolves #115

hera/debugger/debugger.py

@@ -58,6 +58,28 @@ def get_breakpoints(self) -> Dict[int, str]:
     def set_breakpoint(self, b: int) -> None:
         self.breakpoints[b] = self.get_breakpoint_name(b)
 
+    def next(self, *, step) -> None:
+        if self.finished():
+            return
+
+        if not step and self.program.code[self.vm.pc].original.name == "CALL":
+            calls = self.calls
+            # step=True prevents infinite regress.
+            self.next(step=True)
+            while not self.finished() and self.calls > calls:
+                if self.vm.pc in self.breakpoints:
+                    break
+                self.next(step=True)
+        else:
+            real_ops = self.get_real_ops()
+            for real_op in real_ops:
+                if real_op.name == "CALL":
+                    self.calls += 1
+                elif real_op.name == "RETURN":
+                    self.calls -= 1
+
+                real_op.execute(self.vm)
+
     def exec_ops(self, n=-1, *, until=None) -> None:
         """Execute `n` real instructions of the program. If `until` is provided, it
         should be a function that returns True when execution should stop. If `n` is not
@@ -77,7 +99,7 @@ def exec_ops(self, n=-1, *, until=None) -> None:
 
                 real_op.execute(self.vm)
 
-            if self.is_finished() or until(self):
+            if self.finished() or until(self):
                 break
 
             n -= 1
@@ -140,7 +162,7 @@ def find_label(self, ino: int) -> Optional[str]:
                 return symbol
         return None
 
-    def is_finished(self) -> bool:
+    def finished(self) -> bool:
         return self.vm.halted or self.vm.pc >= len(self.program.code)
 
     def empty(self) -> bool:

hera/debugger/shell.py

@@ -334,7 +334,7 @@ def handle_next(self, args):
             print("next takes zero or one arguments.")
             return
 
-        if self.debugger.is_finished():
+        if self.debugger.finished():
             print("Program has finished executing. Enter 'r' to restart.")
             return
 
@@ -344,7 +344,11 @@ def handle_next(self, args):
             print("Could not parse argument to next.")
             return
 
-        self.debugger.exec_ops(n)
+        for _ in range(n):
+            if self.debugger.finished():
+                break
+            self.debugger.next(step=False)
+
         self.print_current_op()
 
     @mutates
@@ -462,8 +466,7 @@ def handle_step(self, args):
             print("step is only valid when the current instruction is CALL.")
             return
 
-        calls = self.debugger.calls
-        self.debugger.exec_ops(until=lambda dbg: dbg.calls == calls)
+        self.debugger.next(step=True)
         self.print_current_op()
 
     def handle_undo(self, args):
@@ -615,7 +618,7 @@ def print_current_op(self):
         """Print the next operation to be executed. If the program has finished
         executed, nothing is printed.
         """
-        if not self.debugger.is_finished():
+        if not self.debugger.finished():
             loc = self.debugger.program.code[self.debugger.vm.pc].loc
             self.print_range_of_ops(loc, context=1)
         else:
@@ -742,7 +745,7 @@ def overflow(v: int) -> bool:
 
     restart         Restart the execution of the program from the beginning.
 
-    step            Step over the execution of a function.
+    step            Step into the execution of a function.
 
     undo            Undo the last operation.
 
@@ -832,8 +835,9 @@ def overflow(v: int) -> bool:
     "next": """\
 next:
   Execute the current line. If the current line is a CALL instruction, the
-  debugger enters the function being called. If you wish to skip over the
-  function call, use `step` instead.
+  debugger executes the entire function (including nested and recursive calls)
+  and moves on to the next line. If you wish to neter over the function call,
+  use `step` instead.
 
 next <n>:
   Execute the next n instructions. This command will follow branches, so be
@@ -874,9 +878,8 @@ def overflow(v: int) -> bool:
     # step
     "step": """\
 step:
-  Step over the execution of a function. The step command is only valid when
-  the current instruction is CALL. Stepping may behave unexpectedly in HERA
-  programs that do not follow conventional function call idioms.""",
+  Step into the execution of a function.  The step command is only valid when
+  the current instruction is CALL.""",
     # undo
     "undo": """\
 undo:

test/test_debugger.py

@@ -128,6 +128,116 @@ def test_handle_next_with_HALT(shell, capsys):
     assert capsys.readouterr().out == "Program has finished executing.\n"
 
 
+def test_handle_next_with_function_call(capsys):
+    shell = load_shell(
+        """\
+SET(R1, 4)
+CALL(FP_alt, plus_two)
+SET(R2, 5)
+HALT()
+
+LABEL(plus_two)
+  INC(R1, 2)
+  RETURN(FP_alt, PC_ret)
+"""
+    )
+    shell.handle_command("n")
+    capsys.readouterr()
+    shell.handle_command("next")
+
+    assert shell.debugger.vm.pc == 5
+    assert shell.debugger.vm.registers[1] == 6
+    assert shell.debugger.vm.registers[2] == 0
+
+    captured = capsys.readouterr().out
+    assert (
+        captured
+        == """\
+[<string>]
+
+    2  CALL(FP_alt, plus_two)
+->  3  SET(R2, 5)
+    4  HALT()
+"""
+    )
+
+
+def test_handle_next_with_recursive_function(capsys):
+    shell = load_shell(
+        """\
+SET(R1, 9)
+CALL(FP_alt, rec)
+HALT()
+
+LABEL(rec)
+  MOVE(FP, SP)
+  INC(SP, 2)
+  STORE(PC_ret, 0, FP)
+  STORE(FP_alt, 1, FP)
+
+  CMP(R1, R0)
+  BZ(end)
+  INC(R2, 1)
+  DEC(R1, 1)
+  CALL(FP_alt, rec)
+  LABEL(end)
+
+  LOAD(PC_ret, 0, FP)
+  LOAD(FP_alt, 1, FP)
+  DEC(SP, 2)
+  RETURN(FP_alt, PC_ret)
+"""
+    )
+    shell.handle_command("n")
+    capsys.readouterr()
+    shell.handle_command("next")
+
+    assert shell.debugger.vm.pc == 5
+    assert shell.debugger.vm.registers[1] == 0
+    assert shell.debugger.vm.registers[2] == 9
+
+    captured = capsys.readouterr().out
+    assert (
+        captured
+        == """\
+[<string>]
+
+     2  CALL(FP_alt, rec)
+->   3  HALT()
+     4
+"""
+    )
+
+
+def test_handle_next_over_function_with_breakpoint(capsys):
+    shell = load_shell(
+        """\
+CALL(FP_alt, foo)
+HALT()
+
+LABEL(foo)
+  RETURN(FP_alt, PC_ret)
+  """
+    )
+    shell.handle_command("break foo")
+    capsys.readouterr()
+    shell.handle_command("next")
+
+    assert shell.debugger.vm.pc == 4
+
+    captured = capsys.readouterr().out
+    assert (
+        captured
+        == """\
+[<string>]
+
+    4  LABEL(foo)
+->  5    RETURN(FP_alt, PC_ret)
+    6
+"""
+    )
+
+
 def test_handle_next_after_end_of_program(shell, capsys):
     shell.debugger.vm.pc = 9000
 
@@ -1004,8 +1114,8 @@ def test_handle_step(capsys):
     capsys.readouterr()
     shell.handle_command("step")
 
-    assert shell.debugger.vm.pc == 5
-    assert shell.debugger.vm.registers[1] == 6
+    assert shell.debugger.vm.pc == 8
+    assert shell.debugger.vm.registers[1] == 4
     assert shell.debugger.vm.registers[2] == 0
 
     captured = capsys.readouterr().out
@@ -1014,56 +1124,9 @@ def test_handle_step(capsys):
         == """\
 [<string>]
 
-    2  CALL(FP_alt, plus_two)
-->  3  SET(R2, 5)
-    4  HALT()
-"""
-    )
-
-
-def test_handle_step_with_recursive_function(capsys):
-    shell = load_shell(
-        """\
-SET(R1, 9)
-CALL(FP_alt, rec)
-HALT()
-
-LABEL(rec)
-  MOVE(FP, SP)
-  INC(SP, 2)
-  STORE(PC_ret, 0, FP)
-  STORE(FP_alt, 1, FP)
-
-  CMP(R1, R0)
-  BZ(end)
-  INC(R2, 1)
-  DEC(R1, 1)
-  CALL(FP_alt, rec)
-  LABEL(end)
-
-  LOAD(PC_ret, 0, FP)
-  LOAD(FP_alt, 1, FP)
-  DEC(SP, 2)
-  RETURN(FP_alt, PC_ret)
-"""
-    )
-    shell.handle_command("n")
-    capsys.readouterr()
-    shell.handle_command("step")
-
-    assert shell.debugger.vm.pc == 5
-    assert shell.debugger.vm.registers[1] == 0
-    assert shell.debugger.vm.registers[2] == 9
-
-    captured = capsys.readouterr().out
-    assert (
-        captured
-        == """\
-[<string>]
-
-     2  CALL(FP_alt, rec)
-->   3  HALT()
-     4
+    6  LABEL(plus_two)
+->  7    INC(R1, 2)
+    8    RETURN(FP_alt, PC_ret)
 """
     )