From cf6f67b5022b58aeda693437ad8447bde37ba684 Mon Sep 17 00:00:00 2001
From: adri09070 <97704417+adri09070@users.noreply.github.com>
Date: Wed, 14 Jun 2023 09:23:14 +0200
Subject: [PATCH] adding tests for skipping returns + making skipReturnNode
 step to first interesting bytecode after having skipped the return node

---
 Sindarin-Tests/SindarinDebuggerTest.class.st | 58 ++++++++++++++++++++
 Sindarin/SindarinDebugger.class.st           |  9 ++-
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/Sindarin-Tests/SindarinDebuggerTest.class.st b/Sindarin-Tests/SindarinDebuggerTest.class.st
index a272616..dd2d992 100644
--- a/Sindarin-Tests/SindarinDebuggerTest.class.st
+++ b/Sindarin-Tests/SindarinDebuggerTest.class.st
@@ -145,6 +145,18 @@ SindarinDebuggerTest >> methodWithSeveralInstructionsInBlock [
 	^ 42
 ]
 
+{ #category : #helpers }
+SindarinDebuggerTest >> methodWithSeveralReturns [
+	"There is only one explicit return  but there is also an implicit return after `a := 3`. In that sense, there are several returns in this method"
+
+	| a |
+	a := true.
+	a
+		ifFalse: [ ^ a := 1 ]
+		ifTrue: [ a := 2 ].
+	a := 3
+]
+
 { #category : #helpers }
 SindarinDebuggerTest >> methodWithTwoAssignments [
 
@@ -1279,6 +1291,52 @@ SindarinDebuggerTest >> testSkipBlockNode [
 	self assert: scdbg topStack equals: 43
 ]
 
+{ #category : #tests }
+SindarinDebuggerTest >> testSkipCanSkipReturnIfItIsNotTheLastReturn [
+
+	| scdbg |
+	scdbg := SindarinDebugger debug: [ self methodWithSeveralReturns ].
+
+	"we step until we arrive on the node `^ a := 1` in `SindarinDebuggerTest>>#methodWithSeveralReturns`."
+	scdbg
+		step;
+		skip;
+		skip;
+		step.
+
+	self assert: scdbg node isReturn.
+	self assert: scdbg topStack equals: 1.
+
+	"We skip the return node"
+	self shouldnt: [ scdbg skip ] raise: SindarinSkippingReturnWarning.
+
+	"We should be on the `a := 2` node"
+	self assert: scdbg node isAssignment.
+	self assert: scdbg node value value equals: 2
+]
+
+{ #category : #tests }
+SindarinDebuggerTest >> testSkipCannotSkipReturnIfItIsTheLastReturn [
+
+	| scdbg nodeWithImplicitReturn |
+	scdbg := SindarinDebugger debug: [ self methodWithSeveralReturns ].
+
+	"we step until we arrive on the method node in `SindarinDebuggerTest>>#methodWithSeveralReturns`, which is mapped to the implicit return bycode."
+	scdbg step.
+	nodeWithImplicitReturn := scdbg methodNode.
+	3 timesRepeat: [ scdbg step ].
+
+	self assert: scdbg node identicalTo: nodeWithImplicitReturn.
+	self assert: scdbg context instructionStream willReturn.
+
+	"We skip the return node"
+	self should: [ scdbg skip ] raise: SindarinSkippingReturnWarning.
+
+	"We should still be on the method node"
+	self assert: scdbg node identicalTo: nodeWithImplicitReturn.
+	self assert: scdbg context instructionStream willReturn
+]
+
 { #category : #tests }
 SindarinDebuggerTest >> testSkipDoesNotSkipReturn [
 
diff --git a/Sindarin/SindarinDebugger.class.st b/Sindarin/SindarinDebugger.class.st
index 4ee565e..50d7573 100644
--- a/Sindarin/SindarinDebugger.class.st
+++ b/Sindarin/SindarinDebugger.class.st
@@ -686,7 +686,7 @@ SindarinDebugger >> skipMessageNodeWith: replacementValue [
 { #category : #'stepping - skip' }
 SindarinDebugger >> skipReturnNode [
 
-	| node allReturnNodes |
+	| node allReturnNodes currentBytecode |
 	node := self node.
 
 	"We collect the list of nodes associated to a return bytecode, via the IR"
@@ -699,7 +699,12 @@ SindarinDebugger >> skipReturnNode [
 	"If this is the last node of the method that is mapped to a return bytecode, we can't skip it and we stop there."
 	node == allReturnNodes last ifTrue: [
 		^ SindarinSkippingReturnWarning signal:
-			  'Cannot skip the last return in the method' ]
+			  'Cannot skip the last return in the method' ].
+
+	currentBytecode := self nextBytecode.
+	self context pc: self context pc + currentBytecode bytes size.
+	self debugSession stepToFirstInterestingBytecodeWithJumpIn:
+		self debugSession interruptedProcess
 ]
 
 { #category : #'stepping -  skip' }