From 113c299a956f23ba1e74d9007022762a4fd3e9d6 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 00:29:56 -0700
Subject: [PATCH 01/14] ALL_CAPS are now constants - manual tested for all but
 recursion which is oddly not triggering

---
 eval/eval.go          | 20 +++++++++++++-------
 eval/eval_test.go     |  4 ++++
 object/object_test.go | 21 +++++++++++++++++++++
 object/state.go       | 25 ++++++++++++++++++++++++-
 4 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/eval/eval.go b/eval/eval.go
index 9ee39333..19b78ebb 100644
--- a/eval/eval.go
+++ b/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)
 }
 
 func ArgCheck[T any](msg string, n int, vararg bool, args []T) *object.Error {
@@ -103,13 +102,12 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 	}
 	switch val := val.(type) {
 	case object.Integer:
-		s.env.Set(id, object.Integer{Value: val.Value + toAdd})
+		return s.env.Set(id, object.Integer{Value: val.Value + toAdd})
 	case object.Float:
-		s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)})
+		return s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)})
 	default:
 		return object.Error{Value: "can't increment/decrement " + val.Type().String()}
 	}
-	return val
 }
 
 // Doesn't unwrap return - return bubbles up.
@@ -174,7 +172,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
+			}
 		}
 		return fn
 	case *ast.CallExpression:
@@ -459,7 +460,12 @@ 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 { // TODO: doesn't trigger
+			oe, _ := oerr.(object.Error)
+			return nil, &oe
+		}
 	}
 	if fn.Variadic {
 		env.Set("..", extra)
diff --git a/eval/eval_test.go b/eval/eval_test.go
index 7513e6be..03a101a5 100644
--- a/eval/eval_test.go
+++ b/eval/eval_test.go
@@ -231,6 +231,10 @@ func TestErrorHandling(t *testing.T) {
 		input           string
 		expectedMessage string
 	}{
+		{
+			`func x(FOO){log("x",FOO,PI)if FOO<=1{return FOO}FOO+x(FOO-1)};FOO(1)`,
+			"constant FOO",
+		},
 		{
 			"myfunc=func(x,y) {x+y}; myfunc(1)",
 			"wrong number of arguments for myfunc. got=1, want=2",
diff --git a/object/object_test.go b/object/object_test.go
index 624f2dca..db0fe462 100644
--- a/object/object_test.go
+++ b/object/object_test.go
@@ -53,3 +53,24 @@ func TestExtensionUsage(t *testing.T) {
 		t.Errorf("cmd.Inspect() test unlimited args got %q, expected %q", actual, expected)
 	}
 }
+
+func TestIsConstantIdentifier(t *testing.T) {
+	tests := []struct {
+		name     string
+		expected bool
+	}{
+		{"PI", true},
+		{"FOO_BAR", true},
+		{"_FOO_BAR", false},
+		{"Foo", false},
+		{"E", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := object.IsConstantIdentifier(tt.name)
+			if actual != tt.expected {
+				t.Errorf("object.IsConstantIdentifier(%q) got %v, expected %v", tt.name, actual, tt.expected)
+			}
+		})
+	}
+}
diff --git a/object/state.go b/object/state.go
index ee92fa56..2fca6210 100644
--- a/object/state.go
+++ b/object/state.go
@@ -1,6 +1,10 @@
 package object
 
-import "fortio.org/log"
+import (
+	"fmt"
+
+	"fortio.org/log"
+)
 
 type Environment struct {
 	store map[string]Object
@@ -28,7 +32,26 @@ func (e *Environment) Get(name string) (Object, bool) {
 	return e.outer.Get(name) // recurse.
 }
 
+func IsConstantIdentifier(name string) bool {
+	for i, v := range name {
+		if i != 0 && v == '_' {
+			continue
+		}
+		if v < 'A' || v > 'Z' {
+			return false
+		}
+	}
+	return true
+}
+
 func (e *Environment) Set(name string, val Object) Object {
+	if IsConstantIdentifier(name) {
+		old, ok := e.Get(name) // not ok
+		if ok {
+			log.Warnf("Attempt to change constant %s from %v to %v", name, old, val)
+			return Error{Value: fmt.Sprintf("attempt to change constant %s from %s to %s", name, old.Inspect(), val.Inspect())}
+		}
+	}
 	e.store[name] = val
 	return val
 }

From ce37333ecf8d31170edd0856e817c66231d013ee Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 08:30:41 -0700
Subject: [PATCH 02/14] Add comments, rename IsConstantIdentifier-> more go
 style Constant()

---
 eval/eval.go          | 6 +++---
 object/object_test.go | 2 +-
 object/state.go       | 6 ++++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/eval/eval.go b/eval/eval.go
index 19b78ebb..b9499824 100644
--- a/eval/eval.go
+++ b/eval/eval.go
@@ -68,7 +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())
-	return s.env.Set(id.Literal(), right)
+	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 {
@@ -104,7 +104,7 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 	case object.Integer:
 		return s.env.Set(id, object.Integer{Value: val.Value + toAdd})
 	case object.Float:
-		return s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)})
+		return 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()}
 	}
@@ -174,7 +174,7 @@ func (s *State) evalInternal(node any) object.Object {
 		if name != nil {
 			oerr := s.env.Set(name.Literal(), fn)
 			if oerr.Type() == object.ERROR {
-				return oerr
+				return oerr // propagate that func FOO() { ... } can only be defined once.
 			}
 		}
 		return fn
diff --git a/object/object_test.go b/object/object_test.go
index db0fe462..9dd31d22 100644
--- a/object/object_test.go
+++ b/object/object_test.go
@@ -67,7 +67,7 @@ func TestIsConstantIdentifier(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			actual := object.IsConstantIdentifier(tt.name)
+			actual := object.Constant(tt.name)
 			if actual != tt.expected {
 				t.Errorf("object.IsConstantIdentifier(%q) got %v, expected %v", tt.name, actual, tt.expected)
 			}
diff --git a/object/state.go b/object/state.go
index 2fca6210..08e31a27 100644
--- a/object/state.go
+++ b/object/state.go
@@ -32,7 +32,9 @@ func (e *Environment) Get(name string) (Object, bool) {
 	return e.outer.Get(name) // recurse.
 }
 
-func IsConstantIdentifier(name string) bool {
+// Defines constant as all CAPS (with _ ok in the middle) identifiers.
+// Note that we use []byte as all identifiers are ASCII.
+func Constant(name string) bool {
 	for i, v := range name {
 		if i != 0 && v == '_' {
 			continue
@@ -45,7 +47,7 @@ func IsConstantIdentifier(name string) bool {
 }
 
 func (e *Environment) Set(name string, val Object) Object {
-	if IsConstantIdentifier(name) {
+	if Constant(name) {
 		old, ok := e.Get(name) // not ok
 		if ok {
 			log.Warnf("Attempt to change constant %s from %v to %v", name, old, val)

From 05bf8b9bb3d197477b5217427d6f5576d0a72037 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 10:25:44 -0700
Subject: [PATCH 03/14] fix mixup between macro env and eval state env; add
 tests for constant; allow resetting to same exact value; tinygo workaround
 for https://github.com/tinygo-org/tinygo/issues/4382

---
 eval/eval_test.go            | 33 +++++++++++++++++++++++++++++++--
 eval/macro_expension.go      | 18 +++++++++---------
 eval/macro_expension_test.go | 17 ++++++++---------
 main.go                      |  7 ++++---
 object/interp.go             |  9 +++++++--
 object/state.go              | 12 ++++++++++--
 repl/repl.go                 | 20 ++++++++++----------
 7 files changed, 79 insertions(+), 37 deletions(-)

diff --git a/eval/eval_test.go b/eval/eval_test.go
index 03a101a5..20a4dde6 100644
--- a/eval/eval_test.go
+++ b/eval/eval_test.go
@@ -11,6 +11,13 @@ import (
 	"grol.io/grol/parser"
 )
 
+func init() {
+	err := extensions.Init()
+	if err != nil {
+		panic(err)
+	}
+}
+
 func TestEvalIntegerExpression(t *testing.T) {
 	tests := []struct {
 		input    string
@@ -59,12 +66,14 @@ 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.
 	}
 	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)
 		}
 	}
 }
@@ -232,9 +241,29 @@ func TestErrorHandling(t *testing.T) {
 		expectedMessage string
 	}{
 		{
-			`func x(FOO){log("x",FOO,PI)if FOO<=1{return FOO}FOO+x(FOO-1)};FOO(1)`,
+			`func x(FOO) {log("x",FOO,PI);if FOO<=1 {return FOO}FOO+x(FOO-1)};FOO(1)`,
 			"constant FOO",
 		},
+		{
+			`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",
diff --git a/eval/macro_expension.go b/eval/macro_expension.go
index 894fcf90..089ed37c 100644
--- a/eval/macro_expension.go
+++ b/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,7 +38,7 @@ 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)
@@ -46,20 +46,20 @@ func (s *State) addMacro(stmt ast.Node) {
 
 	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
 		}
diff --git a/eval/macro_expension_test.go b/eval/macro_expension_test.go
index a9f24342..f31ff6ab 100644
--- a/eval/macro_expension_test.go
+++ b/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)
diff --git a/main.go b/main.go
index 3d9c2739..cd8d51df 100644
--- a/main.go
+++ b/main.go
@@ -10,6 +10,7 @@ import (
 	"fortio.org/log"
 	"grol.io/grol/eval"
 	"grol.io/grol/extensions" // register extensions
+	"grol.io/grol/object"
 	"grol.io/grol/repl"
 )
 
@@ -52,19 +53,19 @@ func Main() int {
 	}
 	options.All = true
 	s := eval.NewState()
-	macroState := eval.NewState()
+	macroState := object.NewMacroEnvironment()
 	for _, file := range flag.Args() {
 		processOneFile(file, s, macroState, options)
 		if !*sharedState {
 			s = eval.NewState()
-			macroState = eval.NewState()
+			macroState = object.NewMacroEnvironment()
 		}
 	}
 	log.Infof("All done")
 	return 0
 }
 
-func processOneFile(file string, s, macroState *eval.State, options repl.Options) {
+func processOneFile(file string, s *eval.State, macroState *object.Environment, options repl.Options) {
 	if file == "-" {
 		if options.FormatOnly {
 			log.Infof("Formatting stdin")
diff --git a/object/interp.go b/object/interp.go
index b99fc19d..e5b8f97a 100644
--- a/object/interp.go
+++ b/object/interp.go
@@ -2,7 +2,6 @@ package object
 
 import (
 	"errors"
-	"maps"
 )
 
 var (
@@ -60,7 +59,13 @@ func initialIdentifiersCopy() map[string]Object {
 	if !initDone {
 		Init()
 	}
-	return maps.Clone(extraIdentifiers)
+	// we'd use maps.Clone except for tinygo not having it.
+	// https://github.com/tinygo-org/tinygo/issues/4382
+	copied := make(map[string]Object, len(extraIdentifiers))
+	for k, v := range extraIdentifiers {
+		copied[k] = v
+	}
+	return copied
 }
 
 func Unwrap(objs []Object) []any {
diff --git a/object/state.go b/object/state.go
index 08e31a27..fe10b957 100644
--- a/object/state.go
+++ b/object/state.go
@@ -11,6 +11,12 @@ type Environment struct {
 	outer *Environment
 }
 
+// Truly empty store suitable for macros storage.
+func NewMacroEnvironment() *Environment {
+	return &Environment{store: make(map[string]Object)}
+}
+
+// NewRootEnvironment contains the identifiers pre-seeded by extensions.
 func NewRootEnvironment() *Environment {
 	s := initialIdentifiersCopy()
 	return &Environment{store: s}
@@ -50,8 +56,10 @@ func (e *Environment) Set(name string, val Object) Object {
 	if Constant(name) {
 		old, ok := e.Get(name) // not ok
 		if ok {
-			log.Warnf("Attempt to change constant %s from %v to %v", name, old, val)
-			return Error{Value: fmt.Sprintf("attempt to change constant %s from %s to %s", name, old.Inspect(), val.Inspect())}
+			log.Debugf("Attempt to change constant %s from %v to %v", name, old, val)
+			if !Hashable(old) || !Hashable(val) || old != val {
+				return Error{Value: fmt.Sprintf("attempt to change constant %s from %s to %s", name, old.Inspect(), val.Inspect())}
+			}
 		}
 	}
 	e.store[name] = val
diff --git a/repl/repl.go b/repl/repl.go
index d6d3d9b8..17850add 100644
--- a/repl/repl.go
+++ b/repl/repl.go
@@ -41,7 +41,7 @@ type Options struct {
 	Compact    bool
 }
 
-func EvalAll(s, macroState *eval.State, in io.Reader, out io.Writer, options Options) {
+func EvalAll(s *eval.State, macroState *object.Environment, in io.Reader, out io.Writer, options Options) {
 	b, err := io.ReadAll(in)
 	if err != nil {
 		log.Fatalf("%v", err)
@@ -65,7 +65,7 @@ func EvalString(what string) (res string, errs []string, formatted string) {
 		}
 	}()
 	s := eval.NewState()
-	macroState := eval.NewState()
+	macroState := object.NewMacroEnvironment()
 	out := &strings.Builder{}
 	s.Out = out
 	s.LogOut = out
@@ -78,7 +78,7 @@ func EvalString(what string) (res string, errs []string, formatted string) {
 
 func Interactive(in io.Reader, out io.Writer, options Options) {
 	s := eval.NewState()
-	macroState := eval.NewState()
+	macroState := object.NewMacroEnvironment()
 
 	scanner := bufio.NewScanner(in)
 	prev := ""
@@ -105,14 +105,14 @@ func Interactive(in io.Reader, out io.Writer, options Options) {
 // Alternate API for benchmarking and simplicity.
 type Grol struct {
 	State     *eval.State
-	Macros    *eval.State
+	Macros    *object.Environment
 	PrintEval bool
 	program   *ast.Statements
 }
 
 // Initialize with new empty state.
 func New() *Grol {
-	g := &Grol{State: eval.NewState(), Macros: eval.NewState()}
+	g := &Grol{State: eval.NewState(), Macros: object.NewMacroEnvironment()}
 	return g
 }
 
@@ -126,7 +126,7 @@ func (g *Grol) Parse(inp []byte) error {
 	if g.Macros == nil {
 		return nil
 	}
-	g.Macros.DefineMacros(g.program)
+	eval.DefineMacros(g.Macros, g.program)
 	numMacros := g.Macros.Len()
 	if numMacros == 0 {
 		return nil
@@ -134,7 +134,7 @@ func (g *Grol) Parse(inp []byte) error {
 	log.LogVf("Expanding, %d macros defined", numMacros)
 	// This actually modifies the original program, not sure... that's good but that's why
 	// expanded return value doesn't need to be used.
-	_ = g.Macros.ExpandMacros(g.program)
+	_ = eval.ExpandMacros(g.Macros, g.program)
 	return nil
 }
 
@@ -152,7 +152,7 @@ func (g *Grol) Run(out io.Writer) error {
 
 // Returns true in line mode if more should be fed to the parser.
 // TODO: this one size fits 3 different calls (file, interactive, bot) is getting spaghetti.
-func EvalOne(s, macroState *eval.State, what string, out io.Writer, options Options) (bool, []string, string) {
+func EvalOne(s *eval.State, macroState *object.Environment, what string, out io.Writer, options Options) (bool, []string, string) {
 	var l *lexer.Lexer
 	if options.All {
 		l = lexer.New(what)
@@ -180,13 +180,13 @@ func EvalOne(s, macroState *eval.State, what string, out io.Writer, options Opti
 			fmt.Fprintln(out)
 		}
 	}
-	macroState.DefineMacros(program)
+	eval.DefineMacros(macroState, program)
 	numMacros := macroState.Len()
 	if numMacros > 0 {
 		log.LogVf("Expanding, %d macros defined", numMacros)
 		// This actually modifies the original program, not sure... that's good but that's why
 		// expanded return value doesn't need to be used.
-		_ = macroState.ExpandMacros(program)
+		_ = eval.ExpandMacros(macroState, program)
 		if options.ShowParse {
 			fmt.Fprint(out, "== Macro ==> ")
 			program.PrettyPrint(&ast.PrintState{Out: out})

From ac2a440b2ade27ea44547ad628c0136db7c71e5b Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 10:29:32 -0700
Subject: [PATCH 04/14] fix foo++ returning foo+1 instead of foo (only return
 error if FOO++)

---
 eval/eval.go | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/eval/eval.go b/eval/eval.go
index b9499824..a15b4402 100644
--- a/eval/eval.go
+++ b/eval/eval.go
@@ -100,14 +100,19 @@ 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:
-		return s.env.Set(id, object.Integer{Value: val.Value + toAdd})
+		oerr = s.env.Set(id, object.Integer{Value: val.Value + toAdd})
 	case object.Float:
-		return s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)}) // So PI++ fails not silently.
+		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.

From fcb5f1f80089699912145b759a9f66c68cb357b3 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 10:33:31 -0700
Subject: [PATCH 05/14] fix usage of N in pi.gr; log setting constant as info

---
 examples/pi.gr  | 6 +++---
 object/state.go | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/examples/pi.gr b/examples/pi.gr
index 2982d9ca..c7b40299 100644
--- a/examples/pi.gr
+++ b/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
diff --git a/object/state.go b/object/state.go
index fe10b957..83c4646f 100644
--- a/object/state.go
+++ b/object/state.go
@@ -56,7 +56,7 @@ func (e *Environment) Set(name string, val Object) Object {
 	if Constant(name) {
 		old, ok := e.Get(name) // not ok
 		if ok {
-			log.Debugf("Attempt to change constant %s from %v to %v", name, old, val)
+			log.Infof("Attempt to change constant %s from %v to %v", name, old, val)
 			if !Hashable(old) || !Hashable(val) || old != val {
 				return Error{Value: fmt.Sprintf("attempt to change constant %s from %s to %s", name, old.Inspect(), val.Inspect())}
 			}

From 43f20dcddc16a74c079135a5e88ed1d963b4f2ca Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 11:36:26 -0700
Subject: [PATCH 06/14] big change in recursion environment, see comments and
 https://github.com/grol-io/grol/issues/47#issuecomment-2267631006

todo: might have messed up the rebase conflicts
---
 eval/eval.go      | 16 ++++++++++++----
 eval/eval_test.go |  5 +++--
 object/state.go   | 21 +++++++++++++++++++--
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/eval/eval.go b/eval/eval.go
index a15b4402..4d15640c 100644
--- a/eval/eval.go
+++ b/eval/eval.go
@@ -421,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
 	}
@@ -441,8 +441,16 @@ 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)
+	env := object.NewFunctionEnvironment(fn, currrentEnv)
 	params := fn.Parameters
 	atLeast := ""
 	extra := object.Array{}
@@ -467,7 +475,7 @@ func extendFunctionEnv(name string, fn object.Function, args []object.Object) (*
 	for paramIdx, param := range params {
 		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 { // TODO: doesn't trigger
+		if oerr.Type() == object.ERROR {
 			oe, _ := oerr.(object.Error)
 			return nil, &oe
 		}
diff --git a/eval/eval_test.go b/eval/eval_test.go
index 20a4dde6..120f0037 100644
--- a/eval/eval_test.go
+++ b/eval/eval_test.go
@@ -68,6 +68,7 @@ func(n) {
 `, 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
 	}
 	for i, tt := range tests {
 		evaluated := testEval(t, tt.input)
@@ -241,8 +242,8 @@ func TestErrorHandling(t *testing.T) {
 		expectedMessage string
 	}{
 		{
-			`func x(FOO) {log("x",FOO,PI);if FOO<=1 {return FOO}FOO+x(FOO-1)};FOO(1)`,
-			"constant FOO",
+			`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}`,
diff --git a/object/state.go b/object/state.go
index 83c4646f..953c0f24 100644
--- a/object/state.go
+++ b/object/state.go
@@ -7,8 +7,9 @@ import (
 )
 
 type Environment struct {
-	store map[string]Object
-	outer *Environment
+	store    map[string]Object
+	outer    *Environment
+	cacheKey string
 }
 
 // Truly empty store suitable for macros storage.
@@ -70,3 +71,19 @@ func NewEnclosedEnvironment(outer *Environment) *Environment {
 	env := &Environment{store: make(map[string]Object), outer: outer}
 	return env
 }
+
+// Create a new environment either based on original function definitions' environment
+// or the current one if the function is the same, that allows a function to set some values
+// visible through recursion to itself.
+//
+//	func test(n) {if (n==2) {x=1}; if (n==1) {return x}; test(n-1)}; test(3)
+//
+// will return 1 (and not "identifier not found: x").
+func NewFunctionEnvironment(fn Function, current *Environment) *Environment {
+	parent := current
+	if current.cacheKey != fn.CacheKey {
+		parent = fn.Env
+	}
+	env := &Environment{store: make(map[string]Object), outer: parent, cacheKey: fn.CacheKey}
+	return env
+}

From 6be89df94e2abddd4110b81a3b90b2a86c70291d Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 11:46:31 -0700
Subject: [PATCH 07/14] add test for presence/absence of == Macro ==> step

---
 main_test.txtar | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/main_test.txtar b/main_test.txtar
index 1370e295..42cd2881 100644
--- a/main_test.txtar
+++ b/main_test.txtar
@@ -37,7 +37,18 @@ stderr 'called fact 5'
 stderr 'called fact 1'
 stderr 'I] All done'
 
-# fib_50.gr
+# macro output (there is a macro in sample_test.gr, it should show that stage with -parse)
+grol -parse sample_test.gr
+!stderr 'Errors'
+stdout '== Macro ==>'
+
+# no macro stage when no macro in the file:
+grol -parse fib_50.gr
+!stderr 'Errors'
+!stdout '== Macro ==>'
+stdout '12586269025\n'
+
+# fib_50.gr (redoing, checking exact match of output)
 grol fib_50.gr
 !stderr 'Errors'
 cmp stdout fib50_stdout

From 39732844d7a42c9c3cea5ce15d3770c6a7706bba Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 12:42:03 -0700
Subject: [PATCH 08/14] linters

---
 eval/eval.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eval/eval.go b/eval/eval.go
index 4d15640c..6cb83bb2 100644
--- a/eval/eval.go
+++ b/eval/eval.go
@@ -116,7 +116,7 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 }
 
 // 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:

From efe013f418c8a70cdaa12fab8b96e2b053837a77 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 13:03:36 -0700
Subject: [PATCH 09/14] Adding examples showing huge perf regression

---
 examples/pi_perf.gr | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 examples/pi_perf.gr

diff --git a/examples/pi_perf.gr b/examples/pi_perf.gr
new file mode 100644
index 00000000..dcc66c97
--- /dev/null
+++ b/examples/pi_perf.gr
@@ -0,0 +1,9 @@
+// Somehow this version works fine with grol 0.24, 0.31 but not at all in current - keeping for regression
+f = func(i, n, prod) {
+	if (i == n+1) {
+		return 1. / (prod * prod * n)
+	}
+	f(i+1, n, prod*(1-1./(2*i)))
+}
+n = 20000
+f(1, n, 1)

From 542cc2de1c21bd0bd63aa824ad5e476ba18c0687 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 15:09:51 -0700
Subject: [PATCH 10/14] adding pprof option to main() and found the issue with
 recursion not using self

---
 eval/eval.go        | 14 ++++++++++----
 eval/eval_test.go   |  4 ++++
 examples/pi_perf.gr |  5 +++--
 main.go             | 31 ++++++++++++++++++++++++++++++-
 object/state.go     | 17 ++++++++++++-----
 5 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/eval/eval.go b/eval/eval.go
index 6cb83bb2..812e695f 100644
--- a/eval/eval.go
+++ b/eval/eval.go
@@ -448,9 +448,11 @@ func extendFunctionEnv(
 ) (*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)
+	//     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)
-	env := object.NewFunctionEnvironment(fn, currrentEnv)
+	// 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{}
@@ -481,11 +483,15 @@ func extendFunctionEnv(
 		}
 	}
 	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
 }
 
diff --git a/eval/eval_test.go b/eval/eval_test.go
index 120f0037..4ae47b53 100644
--- a/eval/eval_test.go
+++ b/eval/eval_test.go
@@ -69,6 +69,10 @@ func(n) {
 		{`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)
diff --git a/examples/pi_perf.gr b/examples/pi_perf.gr
index dcc66c97..3bec0353 100644
--- a/examples/pi_perf.gr
+++ b/examples/pi_perf.gr
@@ -1,9 +1,10 @@
-// Somehow this version works fine with grol 0.24, 0.31 but not at all in current - keeping for regression
+// 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 = 20000
+n = 100000
 f(1, n, 1)
diff --git a/main.go b/main.go
index cd8d51df..32caddfc 100644
--- a/main.go
+++ b/main.go
@@ -5,6 +5,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"runtime/pprof"
 
 	"fortio.org/cli"
 	"fortio.org/log"
@@ -25,6 +26,9 @@ func Main() int {
 	compact := flag.Bool("compact", false, "When printing code, use no indentation and most compact form")
 	showEval := flag.Bool("eval", true, "show eval results")
 	sharedState := flag.Bool("shared-state", false, "All files share same interpreter state (default is new state for each)")
+	cpuprofile := flag.String("profile-cpu", "", "write cpu profile to `file`")
+	memprofile := flag.String("profile-mem", "", "write memory profile to `file`")
+
 	cli.ArgsHelp = "*.gr files to interpret or `-` for stdin without prompt or no arguments for stdin repl..."
 	cli.MaxArgs = -1
 	cli.Main()
@@ -35,9 +39,22 @@ func Main() int {
 		FormatOnly: *format,
 		Compact:    *compact,
 	}
+
+	if *cpuprofile != "" {
+		f, err := os.Create(*cpuprofile)
+		if err != nil {
+			log.Fatalf("can't open file for cpu profile: %v", err)
+		}
+		err = pprof.StartCPUProfile(f)
+		if err != nil {
+			log.Fatalf("can't start cpu profile: %v", err)
+		}
+		log.Infof("Writing cpu profile to %s", *cpuprofile)
+		defer pprof.StopCPUProfile()
+	}
 	err := extensions.Init()
 	if err != nil {
-		log.Fatalf("Error initializing extensions: %v", err)
+		return log.FErrf("Error initializing extensions: %v", err)
 	}
 	if *commandFlag != "" {
 		res, errs, _ := repl.EvalString(*commandFlag)
@@ -62,6 +79,18 @@ func Main() int {
 		}
 	}
 	log.Infof("All done")
+	if *memprofile != "" {
+		f, err := os.Create(*memprofile)
+		if err != nil {
+			return log.FErrf("can't open file for mem profile: %v", err)
+		}
+		err = pprof.WriteHeapProfile(f)
+		if err != nil {
+			return log.FErrf("can't write mem profile: %v", err)
+		}
+		log.Infof("Wrote memory profile to %s", *memprofile)
+		f.Close()
+	}
 	return 0
 }
 
diff --git a/object/state.go b/object/state.go
index 953c0f24..5aef3ad3 100644
--- a/object/state.go
+++ b/object/state.go
@@ -53,6 +53,11 @@ func Constant(name string) bool {
 	return true
 }
 
+func (e *Environment) SetNoChecks(name string, val Object) Object {
+	e.store[name] = val
+	return val
+}
+
 func (e *Environment) Set(name string, val Object) Object {
 	if Constant(name) {
 		old, ok := e.Get(name) // not ok
@@ -63,8 +68,7 @@ func (e *Environment) Set(name string, val Object) Object {
 			}
 		}
 	}
-	e.store[name] = val
-	return val
+	return e.SetNoChecks(name, val)
 }
 
 func NewEnclosedEnvironment(outer *Environment) *Environment {
@@ -79,11 +83,14 @@ func NewEnclosedEnvironment(outer *Environment) *Environment {
 //	func test(n) {if (n==2) {x=1}; if (n==1) {return x}; test(n-1)}; test(3)
 //
 // will return 1 (and not "identifier not found: x").
-func NewFunctionEnvironment(fn Function, current *Environment) *Environment {
+// Returns true if the function is the same as the current one and we should probably
+// set the function's name in that environment to avoid deep search for it.
+func NewFunctionEnvironment(fn Function, current *Environment) (*Environment, bool) {
 	parent := current
-	if current.cacheKey != fn.CacheKey {
+	sameFunction := (current.cacheKey == fn.CacheKey)
+	if !sameFunction {
 		parent = fn.Env
 	}
 	env := &Environment{store: make(map[string]Object), outer: parent, cacheKey: fn.CacheKey}
-	return env
+	return env, sameFunction
 }

From b0260c189ea441490c99a321ef7dcdf37e0d276e Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Sun, 4 Aug 2024 15:34:26 -0700
Subject: [PATCH 11/14] unrelated fix for wasm/web share: preserve the tabs

---
 wasm/grol_wasm.html | 1 +
 1 file changed, 1 insertion(+)

diff --git a/wasm/grol_wasm.html b/wasm/grol_wasm.html
index 00cab343..42780ccc 100644
--- a/wasm/grol_wasm.html
+++ b/wasm/grol_wasm.html
@@ -128,6 +128,7 @@
             return str.replace(/\+/g, '%2B')
               .replace(/ /g, '+')
               .replace(/\n/g, '%0A')
+              .replace(/\t/g, '%09')
               .replace(/</g, '%3C')
         }
         const encodedValue = customEscape(paramValue)

From 60a489969ad4c0090d1cc915cc0857feebc1f2c0 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Mon, 5 Aug 2024 07:03:56 -0700
Subject: [PATCH 12/14] replace 2 log.Fatalf by return log.FErrf for
 consistency

---
 main.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/main.go b/main.go
index 32caddfc..6df88f9a 100644
--- a/main.go
+++ b/main.go
@@ -43,11 +43,11 @@ func Main() int {
 	if *cpuprofile != "" {
 		f, err := os.Create(*cpuprofile)
 		if err != nil {
-			log.Fatalf("can't open file for cpu profile: %v", err)
+			return log.FErrf("can't open file for cpu profile: %v", err)
 		}
 		err = pprof.StartCPUProfile(f)
 		if err != nil {
-			log.Fatalf("can't start cpu profile: %v", err)
+			return log.FErrf("can't start cpu profile: %v", err)
 		}
 		log.Infof("Writing cpu profile to %s", *cpuprofile)
 		defer pprof.StopCPUProfile()

From 9b8635a9a1f3a95b40e743cdb808cfd043154800 Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Mon, 5 Aug 2024 07:06:50 -0700
Subject: [PATCH 13/14] Apply suggestions from code review

Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
---
 eval/eval.go      | 2 +-
 eval/eval_test.go | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/eval/eval.go b/eval/eval.go
index 812e695f..833d2add 100644
--- a/eval/eval.go
+++ b/eval/eval.go
@@ -447,7 +447,7 @@ func extendFunctionEnv(
 	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
+	// 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
diff --git a/eval/eval_test.go b/eval/eval_test.go
index 27e95c15..81eb0de0 100644
--- a/eval/eval_test.go
+++ b/eval/eval_test.go
@@ -11,11 +11,12 @@ import (
 	"grol.io/grol/parser"
 )
 
-func init() {
+func TestMain(m *testing.M) {	
 	err := extensions.Init()
 	if err != nil {
 		panic(err)
 	}
+	os.Exit(m.Run())
 }
 
 func TestEvalIntegerExpression(t *testing.T) {

From 2d03d452174497ffb60b7c506ed0eb09b7edf16c Mon Sep 17 00:00:00 2001
From: Laurent Demailly <ldemailly@gmail.com>
Date: Mon, 5 Aug 2024 07:08:11 -0700
Subject: [PATCH 14/14] add missing import

---
 eval/eval_test.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/eval/eval_test.go b/eval/eval_test.go
index 81eb0de0..e01b75d6 100644
--- a/eval/eval_test.go
+++ b/eval/eval_test.go
@@ -1,6 +1,7 @@
 package eval_test
 
 import (
+	"os"
 	"testing"
 
 	"grol.io/grol/ast"
@@ -11,7 +12,7 @@ import (
 	"grol.io/grol/parser"
 )
 
-func TestMain(m *testing.M) {	
+func TestMain(m *testing.M) {
 	err := extensions.Init()
 	if err != nil {
 		panic(err)