ALL_CAPS are now constants. Changed/shared recursive function environment. Fixed bug with macro environment. Added pprof.

eval/eval.go

@@ -68,8 +68,7 @@ func (s *State) evalAssignment(right object.Object, node *ast.InfixExpression) o
 		return right
 	}
 	log.LogVf("eval assign %#v to %#v", right, id.Value())
-	s.env.Set(id.Literal(), right)
-	return right // maybe only if it's a literal?
+	return s.env.Set(id.Literal(), right) // Propagate possible error (constant setting).
 }
 
 func ArgCheck[T any](msg string, n int, vararg bool, args []T) *object.Error {
@@ -101,19 +100,23 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 	default:
 		return object.Error{Value: "unknown postfix operator: " + node.Type().String()}
 	}
+	var oerr object.Object
 	switch val := val.(type) {
 	case object.Integer:
-		s.env.Set(id, object.Integer{Value: val.Value + toAdd})
+		oerr = s.env.Set(id, object.Integer{Value: val.Value + toAdd})
 	case object.Float:
-		s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)})
+		oerr = s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)}) // So PI++ fails not silently.
 	default:
 		return object.Error{Value: "can't increment/decrement " + val.Type().String()}
 	}
+	if oerr.Type() == object.ERROR {
+		return oerr
+	}
 	return val
 }
 
 // Doesn't unwrap return - return bubbles up.
-func (s *State) evalInternal(node any) object.Object {
+func (s *State) evalInternal(node any) object.Object { //nolint:funlen // quite a lot of cases.
 	switch node := node.(type) {
 	// Statements
 	case *ast.Statements:
@@ -174,7 +177,10 @@ func (s *State) evalInternal(node any) object.Object {
 		}
 		fn.SetCacheKey() // sets cache key
 		if name != nil {
-			s.env.Set(name.Literal(), fn)
+			oerr := s.env.Set(name.Literal(), fn)
+			if oerr.Type() == object.ERROR {
+				return oerr // propagate that func FOO() { ... } can only be defined once.
+			}
 		}
 		return fn
 	case *ast.CallExpression:
@@ -415,7 +421,7 @@ func (s *State) applyFunction(name string, fn object.Object, args []object.Objec
 		_, _ = s.Out.Write(output)
 		return v
 	}
-	nenv, oerr := extendFunctionEnv(name, function, args)
+	nenv, oerr := extendFunctionEnv(s.env, name, function, args)
 	if oerr != nil {
 		return *oerr
 	}
@@ -435,8 +441,18 @@ func (s *State) applyFunction(name string, fn object.Object, args []object.Objec
 	return res
 }
 
-func extendFunctionEnv(name string, fn object.Function, args []object.Object) (*object.Environment, *object.Error) {
-	env := object.NewEnclosedEnvironment(fn.Env)
+func extendFunctionEnv(
+	currrentEnv *object.Environment,
+	name string, fn object.Function,
+	args []object.Object,
+) (*object.Environment, *object.Error) {
+	// https://github.com/grol-io/grol/issues/47
+	// fn.Env is "captured state", but for recursion we now parent from current state; eg
+	//     func test(n) {if (n==2) {x=1}; if (n==1) {return x}; test(n-1)}; test(3)
+	// return 1 (state set by recursion with n==2)
+	// Make sure `self` is used to recurse, or named function, otherwise the function will
+	// need to be found way up the now much deeper stack.
+	env, sameFunction := object.NewFunctionEnvironment(fn, currrentEnv)
 	params := fn.Parameters
 	atLeast := ""
 	extra := object.Array{}
@@ -459,14 +475,23 @@ func extendFunctionEnv(name string, fn object.Function, args []object.Object) (*
 			name, len(args), atLeast, n)}
 	}
 	for paramIdx, param := range params {
-		env.Set(param.Value().Literal(), args[paramIdx])
+		oerr := env.Set(param.Value().Literal(), args[paramIdx])
+		log.LogVf("set %s to %s - %s", param.Value().Literal(), args[paramIdx].Inspect(), oerr.Inspect())
+		if oerr.Type() == object.ERROR {
+			oe, _ := oerr.(object.Error)
+			return nil, &oe
+		}
 	}
 	if fn.Variadic {
-		env.Set("..", extra)
+		env.SetNoChecks("..", extra)
 	}
 	// for recursion in anonymous functions.
 	// TODO: consider not having to keep setting this in the function's env and treating as a keyword.
-	env.Set("self", fn)
+	env.SetNoChecks("self", fn)
+	// For recursion in named functions, set it here so we don't need to go up a stack of 50k envs to find it
+	if sameFunction && name != "" {
+		env.SetNoChecks(name, fn)
+	}
 	return env, nil
 }
 

eval/eval_test.go

@@ -1,6 +1,7 @@
 package eval_test
 
 import (
+	"os"
 	"testing"
 
 	"grol.io/grol/ast"
@@ -11,6 +12,14 @@ import (
 	"grol.io/grol/parser"
 )
 
+func TestMain(m *testing.M) {
+	err := extensions.Init()
+	if err != nil {
+		panic(err)
+	}
+	os.Exit(m.Run())
+}
+
 func TestEvalIntegerExpression(t *testing.T) {
 	tests := []struct {
 		input    string
@@ -59,12 +68,19 @@ func(n) {
 	n * self(n - 1)
 }(5)
 `, 120},
+		{`ONE=1;ONE`, 1},
+		{`ONE=1;ONE=1`, 1}, // Ok to reassign CONSTANT if it's to same value.
+		{`myid=23; func test(n) {if (n==2) {myid=42}; if (n==1) {return myid}; test(n-1)}; test(3)`, 42}, // was 23 before
+		{
+			`func FACT(n){if n<=1 {return 1}; n*FACT(n-1)};FACT(5)`, // Recursion on CONSTANT function should not error
+			120,
+		},
 	}
 	for i, tt := range tests {
 		evaluated := testEval(t, tt.input)
 		r := testIntegerObject(t, evaluated, tt.expected)
 		if !r {
-			t.Logf("test %d input: %s failed integer %d", i, tt.input, tt.expected)
+			t.Logf("test %d input: %s failed to eval to integer %d", i, tt.input, tt.expected)
 		}
 	}
 }
@@ -231,6 +247,30 @@ func TestErrorHandling(t *testing.T) {
 		input           string
 		expectedMessage string
 	}{
+		{
+			`func x(FOO) {log("x",FOO,PI);if FOO<=1 {return FOO} x(FOO-1)};x(2)`,
+			"attempt to change constant FOO from 2 to 1",
+		},
+		{
+			`func FOO(x){x}; func FOO(x){x+1}`,
+			"attempt to change constant FOO from func FOO(x){x} to func FOO(x){x+1}",
+		},
+		{
+			`ONE=1;ONE=2`,
+			"attempt to change constant ONE from 1 to 2",
+		},
+		{
+			`ONE=1;ONE--`,
+			"attempt to change constant ONE from 1 to 0",
+		},
+		{
+			`PI++`,
+			"attempt to change constant PI from 3.141592653589793 to 4.141592653589793",
+		},
+		{
+			`ONE=1;func f(x){func ff(y) {ONE=y} ff(x)};f(3)`,
+			"attempt to change constant ONE from 1 to 3",
+		},
 		{
 			"myfunc=func(x,y) {x+y}; myfunc(1)",
 			"wrong number of arguments for myfunc. got=1, want=2",

eval/macro_expension.go

@@ -9,11 +9,11 @@ import (
 	"grol.io/grol/token"
 )
 
-func (s *State) DefineMacros(program *ast.Statements) {
+func DefineMacros(store *object.Environment, program *ast.Statements) {
 	for i := 0; i < len(program.Statements); /* not always incrementing */ {
 		statement := program.Statements[i]
 		if isMacroDefinition(statement) {
-			s.addMacro(statement)
+			addMacro(store, statement)
 			program.Statements = append(program.Statements[:i], program.Statements[i+1:]...)
 		} else {
 			i++
@@ -38,28 +38,28 @@ func isMacroDefinition(node ast.Node) bool {
 	return ok
 }
 
-func (s *State) addMacro(stmt ast.Node) {
+func addMacro(s *object.Environment, stmt ast.Node) {
 	// TODO ok checks
 	assign, _ := stmt.(*ast.InfixExpression)
 	macroLiteral, _ := assign.Right.(*ast.MacroLiteral)
 	name := assign.Left.(*ast.Identifier).Literal()
 
 	macro := &object.Macro{
 		Parameters: macroLiteral.Parameters,
-		Env:        s.env,
+		Env:        s,
 		Body:       macroLiteral.Body,
 	}
 
-	s.env.Set(name, macro)
+	s.Set(name, macro)
 }
 
-func (s *State) isMacroCall(exp *ast.CallExpression) (*object.Macro, bool) {
+func isMacroCall(s *object.Environment, exp *ast.CallExpression) (*object.Macro, bool) {
 	identifier, ok := exp.Function.(*ast.Identifier)
 	if !ok {
 		return nil, false
 	}
 
-	obj, ok := s.env.Get(identifier.Literal())
+	obj, ok := s.Get(identifier.Literal())
 	if !ok {
 		return nil, false
 	}
@@ -72,14 +72,14 @@ func (s *State) isMacroCall(exp *ast.CallExpression) (*object.Macro, bool) {
 	return macro, true
 }
 
-func (s *State) ExpandMacros(program ast.Node) ast.Node {
+func ExpandMacros(s *object.Environment, program ast.Node) ast.Node {
 	return ast.Modify(program, func(node ast.Node) ast.Node {
 		callExpression, ok := node.(*ast.CallExpression)
 		if !ok {
 			return node
 		}
 
-		macro, ok := s.isMacroCall(callExpression)
+		macro, ok := isMacroCall(s, callExpression)
 		if !ok {
 			return node
 		}

eval/macro_expension_test.go

@@ -15,27 +15,26 @@ func TestDefineMacros(t *testing.T) {
     mymacro = macro(x, y) { x + y; }
 	number + 1`
 
-	state := NewState()
-	env := state.env
+	state := object.NewMacroEnvironment()
 	program := testParseProgram(t, input)
 
-	state.DefineMacros(program)
+	DefineMacros(state, program)
 
 	if len(program.Statements) != 3 {
 		t.Fatalf("Wrong number of statements. got=%d: %+v",
 			len(program.Statements), program.Statements)
 	}
 
-	_, ok := env.Get("number")
+	_, ok := state.Get("number")
 	if ok {
 		t.Fatalf("number should not be defined")
 	}
-	_, ok = env.Get("function")
+	_, ok = state.Get("function")
 	if ok {
 		t.Fatalf("function should not be defined")
 	}
 
-	obj, ok := env.Get("mymacro")
+	obj, ok := state.Get("mymacro")
 	if !ok {
 		t.Fatalf("macro not in environment.")
 	}
@@ -124,9 +123,9 @@ func TestExpandMacros(t *testing.T) {
 		expected := testParseProgram(t, tt.expected)
 		program := testParseProgram(t, tt.input)
 
-		state := NewState()
-		state.DefineMacros(program)
-		expanded := state.ExpandMacros(program)
+		state := object.NewMacroEnvironment()
+		DefineMacros(state, program)
+		expanded := ExpandMacros(state, program)
 
 		expandedStr := ast.DebugString(expanded)
 		expectedStr := ast.DebugString(expected)

examples/pi.gr

@@ -1,12 +1,12 @@
-f = func(n, fac, dfac, exp, N) {
+f = func(n, fac, dfac, exp, max) {
   // log("Call n=", n, fac, dfac, exp)
-  if (n>N) {
+  if (n>max) {
     [fac, dfac, exp]
   } else {
     dfac = 1.*dfac*(2*n - 1)
     exp = exp * 2
     fac = fac * n
-    f(n+1, fac, dfac, exp, N)
+    f(n+1, fac, dfac, exp, max)
   }
 }
 N = 100

examples/pi_perf.gr

@@ -0,0 +1,10 @@
+// Keep 0.24 syntax for perf comparison. Used to detect a serious performance regression
+// during https://github.com/grol-io/grol/pull/102
+f = func(i, n, prod) {
+	if (i == n+1) {
+		return 1. / (prod * prod * n)
+	}
+	f(i+1, n, prod*(1-1./(2*i)))
+}
+n = 100000
+f(1, n, 1)