Repository: tetratelabs/wazero
Pull request #1716 (2023-09-18 05:21:43)

experimental: removes Parameters API from StackIterator
as discussed offline in the Gophers slack, the Parameters API on 
the experimental StackIterator hasn't been used in practice, and 
also is difficult to implement in the optimizing compiler.
Therefore, we've decided to delete it from the API.



Patch:
diff --git a/experimental/listener.go b/experimental/listener.go
index 7c1ce6fb..64d98e90 100644
--- a/experimental/listener.go
+++ b/experimental/listener.go
@@ -21,10 +21,6 @@ type StackIterator interface {
 	// ProgramCounter returns the program counter associated with the
 	// function call.
 	ProgramCounter() ProgramCounter
-	// Parameters returns api.ValueType-encoded parameters of the current
-	// function. Do not modify the content of the slice, and copy out any
-	// value you need.
-	Parameters() []uint64
 }
 
 // FunctionListenerFactoryKey is a context.Context Value key. Its associated value should be a FunctionListenerFactory.
@@ -194,48 +190,21 @@ func (multi *multiFunctionListener) Abort(ctx context.Context, mod api.Module, d
 	}
 }
 
-type parameters struct {
-	values []uint64
-	limits []uint8
-}
-
-func (ps *parameters) append(values []uint64) {
-	ps.values = append(ps.values, values...)
-	ps.limits = append(ps.limits, uint8(len(ps.values)))
-}
-
-func (ps *parameters) clear() {
-	ps.values = ps.values[:0]
-	ps.limits = ps.limits[:0]
-}
-
-func (ps *parameters) index(i int) []uint64 {
-	j := uint8(0)
-	k := ps.limits[i]
-	if i > 0 {
-		j = ps.limits[i-1]
-	}
-	return ps.values[j:k:k]
-}
-
 type stackIterator struct {
-	base   StackIterator
-	index  int
-	pcs    []uint64
-	fns    []InternalFunction
-	params parameters
+	base  StackIterator
+	index int
+	pcs   []uint64
+	fns   []InternalFunction
 }
 
 func (si *stackIterator) Next() bool {
 	if si.base != nil {
 		si.pcs = si.pcs[:0]
 		si.fns = si.fns[:0]
-		si.params.clear()
 
 		for si.base.Next() {
 			si.pcs = append(si.pcs, uint64(si.base.ProgramCounter()))
 			si.fns = append(si.fns, si.base.Function())
-			si.params.append(si.base.Parameters())
 		}
 
 		si.base = nil
@@ -252,10 +221,6 @@ func (si *stackIterator) Function() InternalFunction {
 	return si.fns[si.index]
 }
 
-func (si *stackIterator) Parameters() []uint64 {
-	return si.params.index(si.index)
-}
-
 // StackFrame represents a frame on the call stack.
 type StackFrame struct {
 	Function     api.Function
@@ -302,10 +267,6 @@ func (si *stackFrameIterator) ProgramCounter() ProgramCounter {
 	return ProgramCounter(si.stack[si.index].PC)
 }
 
-func (si *stackFrameIterator) Parameters() []uint64 {
-	return si.stack[si.index].Params
-}
-
 // NewStackIterator constructs a stack iterator from a list of stack frames.
 // The top most frame is the last one.
 func NewStackIterator(stack ...StackFrame) StackIterator {
diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go
index f8b76fa4..8fc9d52b 100644
--- a/internal/engine/compiler/engine.go
+++ b/internal/engine/compiler/engine.go
@@ -1195,11 +1195,6 @@ func (si *stackIterator) Function() experimental.InternalFunction {
 	return internalFunction{si.fn}
 }
 
-// Parameters implements the same method as documented on experimental.StackIterator.
-func (si *stackIterator) Parameters() []uint64 {
-	return si.stack[si.base : si.base+si.fn.funcType.ParamNumInUint64]
-}
-
 // internalFunction implements experimental.InternalFunction.
 type internalFunction struct{ *function }
 
diff --git a/internal/engine/interpreter/interpreter.go b/internal/engine/interpreter/interpreter.go
index 8f0db3c7..f19f33f3 100644
--- a/internal/engine/interpreter/interpreter.go
+++ b/internal/engine/interpreter/interpreter.go
@@ -271,14 +271,6 @@ func (si *stackIterator) ProgramCounter() experimental.ProgramCounter {
 	return experimental.ProgramCounter(si.pc)
 }
 
-// Parameters implements the same method as documented on
-// experimental.StackIterator.
-func (si *stackIterator) Parameters() []uint64 {
-	paramsCount := si.fn.funcType.ParamNumInUint64
-	top := len(si.stack)
-	return si.stack[top-paramsCount:]
-}
-
 // internalFunction implements experimental.InternalFunction.
 type internalFunction struct{ *function }
 


Test patch:
diff --git a/experimental/listener_example_test.go b/experimental/listener_example_test.go
index 8b7f37d5..57d346cc 100644
--- a/experimental/listener_example_test.go
+++ b/experimental/listener_example_test.go
@@ -94,22 +94,18 @@ func Example_stackIterator() {
 		fn := it.Function()
 		pc := it.ProgramCounter()
 		fmt.Println("function:", fn.Definition().DebugName())
-		fmt.Println("\tparameters:", it.Parameters())
 		fmt.Println("\tprogram counter:", pc)
 		fmt.Println("\tsource offset:", fn.SourceOffsetForPC(pc))
 	}
 
 	// Output:
 	// function: fn0
-	// 	parameters: [1 2 3]
 	// 	program counter: 5890831
 	// 	source offset: 1234
 	// function: fn1
-	// 	parameters: []
 	// 	program counter: 5899822
 	// 	source offset: 7286
 	// function: fn2
-	// 	parameters: [4]
 	// 	program counter: 6820312
 	// 	source offset: 935891
 }
@@ -153,10 +149,6 @@ func (s *fakeStackIterator) Function() experimental.InternalFunction {
 	}
 }
 
-func (s *fakeStackIterator) Parameters() []uint64 {
-	return s.args
-}
-
 func (s *fakeStackIterator) ProgramCounter() experimental.ProgramCounter {
 	return experimental.ProgramCounter(s.pc)
 }
diff --git a/experimental/listener_test.go b/experimental/listener_test.go
index 3db4863f..c634451b 100644
--- a/experimental/listener_test.go
+++ b/experimental/listener_test.go
@@ -125,22 +125,6 @@ func TestMultiFunctionListenerFactory(t *testing.T) {
 		n++
 		i := 0
 		for stackIterator.Next() {
-			var param uint64
-			switch i {
-			case 0:
-				param = 3
-			case 1:
-				param = 2
-			case 2:
-				param = 1
-			default:
-				t.Errorf("too many frames seen by stack iterator: %d", i)
-			}
-			if params := stackIterator.Parameters(); len(params) != 1 {
-				t.Errorf("wrong number of parameters in call frame %d: want=1 got=%d", i, len(params))
-			} else if params[0] != param {
-				t.Errorf("wrong parameter in call frame %d: want=%d got=%d", i, param, params[0])
-			}
 			i++
 		}
 		if i != 3 {
diff --git a/internal/engine/compiler/engine_test.go b/internal/engine/compiler/engine_test.go
index b714f825..c68a0586 100644
--- a/internal/engine/compiler/engine_test.go
+++ b/internal/engine/compiler/engine_test.go
@@ -436,14 +436,13 @@ func Test_callFrameOffset(t *testing.T) {
 }
 
 type stackEntry struct {
-	def  api.FunctionDefinition
-	args []uint64
+	def api.FunctionDefinition
 }
 
 func assertStackIterator(t *testing.T, it experimental.StackIterator, expected []stackEntry) {
 	var actual []stackEntry
 	for it.Next() {
-		actual = append(actual, stackEntry{def: it.Function().Definition(), args: it.Parameters()})
+		actual = append(actual, stackEntry{def: it.Function().Definition()})
 	}
 	require.Equal(t, expected, actual)
 }
@@ -458,7 +457,7 @@ func TestCallEngine_builtinFunctionFunctionListenerBefore(t *testing.T) {
 				before: func(ctx context.Context, _ api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) {
 					require.Equal(t, currentContext, ctx)
 					require.Equal(t, []uint64{2, 3, 4}, params)
-					assertStackIterator(t, stackIterator, []stackEntry{{def: def, args: []uint64{2, 3, 4}}})
+					assertStackIterator(t, stackIterator, []stackEntry{{def: def}})
 				},
 			},
 			index: 0,
diff --git a/internal/integration_test/engine/adhoc_test.go b/internal/integration_test/engine/adhoc_test.go
index 112258f8..ea3e8a3e 100644
--- a/internal/integration_test/engine/adhoc_test.go
+++ b/internal/integration_test/engine/adhoc_test.go
@@ -1329,27 +1329,26 @@ func testBeforeListenerGlobals(t *testing.T, r wazero.Runtime) {
 func testBeforeListenerStackIterator(t *testing.T, r wazero.Runtime) {
 	type stackEntry struct {
 		debugName string
-		args      []uint64
 	}
 
 	expectedCallstacks := [][]stackEntry{
 		{ // when calling f1
-			{debugName: "whatever.f1", args: []uint64{2, 3, 4}},
+			{debugName: "whatever.f1"},
 		},
 		{ // when calling f2
-			{debugName: "whatever.f2", args: []uint64{}},
-			{debugName: "whatever.f1", args: []uint64{2, 3, 4}},
+			{debugName: "whatever.f2"},
+			{debugName: "whatever.f1"},
 		},
-		{ // when calling f3
-			{debugName: "whatever.f3", args: []uint64{5}},
-			{debugName: "whatever.f2", args: []uint64{}},
-			{debugName: "whatever.f1", args: []uint64{2, 3, 4}},
+		{ // when calling
+			{debugName: "whatever.f3"},
+			{debugName: "whatever.f2"},
+			{debugName: "whatever.f1"},
 		},
 		{ // when calling f4
-			{debugName: "host.f4", args: []uint64{6}},
-			{debugName: "whatever.f3", args: []uint64{5}},
-			{debugName: "whatever.f2", args: []uint64{}},
-			{debugName: "whatever.f1", args: []uint64{2, 3, 4}},
+			{debugName: "host.f4"},
+			{debugName: "whatever.f3"},
+			{debugName: "whatever.f2"},
+			{debugName: "whatever.f1"},
 		},
 	}
 
@@ -1360,7 +1359,6 @@ func testBeforeListenerStackIterator(t *testing.T, r wazero.Runtime) {
 			for si.Next() {
 				require.True(t, len(expectedCallstack) > 0)
 				require.Equal(t, expectedCallstack[0].debugName, si.Function().Definition().DebugName())
-				require.Equal(t, expectedCallstack[0].args, si.Parameters())
 				expectedCallstack = expectedCallstack[1:]
 			}
 			require.Equal(t, 0, len(expectedCallstack))