Reference object type: Allow mutations in upper scope, add back := …

…for forcing local scope variable creation; cache bug fix (#206)

* Reference type

* progress: force create and make refs unless := (future) or fn params (in this commit)

* Adding := as local (re)definition/copy. seemingly working version overall

* Adding a few basic tests

* fix the loop/review comment - add failing test

* fix makefile generate, fix ref of ref case

* cover post and pre incr/decr. add example of state in lambda

* Fix reference loops, add tests

* add direct state from params example

* prevent aliases upstack/avoid indirect loops; fix caching bug and add test for it using 2 level deep rand()

* Catch errors like m[xy++] if xy is not incrementable and lay ground work for #189 (failing test)

* Clean up index expression with . so it actually errors out for m.xy++ pending implementation

* Reduce greatly the peppering of object.Value() and return the actual type for REFERENCE instead of referenced object's type

* move evalinternal -> eval but need some negative additional testing that return unwrap isn't now too early

* Adding test of return bubbling up correctly inside for loops and nesting

* copy multi platform fortio workflow (and somehow setup-go action is old here)

* maybe fix testscript on windows

* Give up on testscript and windows for now

* fix bug index assignment missing deref

.github/workflows/gochecks.yml

@@ -10,25 +10,37 @@ on:
 
 jobs:
   check:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest, windows-latest, macos-latest]
     steps:
       - name: Checkout
         uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # pin@v4
       - name: Setup Go environment
-        uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # pin@v5
+        uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # pin@v5
         with:
           go-version: '1.22'
           check-latest: true
       - name: Run Vulncheck
+        if: matrix.os == 'ubuntu-latest'
         run: |
           go install golang.org/x/vuln/cmd/govulncheck@latest
           govulncheck ./...
       - name: Download linter config
+        if: matrix.os == 'ubuntu-latest'
         run: curl -fsS -o .golangci.yml https://raw.githubusercontent.com/fortio/workflows/main/golangci.yml
       - name: Run golangci-lint
+        if: matrix.os == 'ubuntu-latest'
         uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # pin@v6
         with:
           version: v1.60.3
+      - name: Install Git Bash
+        run: |
+          choco install git
+          echo "Git Bash installed"
+        shell: pwsh
+        if: runner.os == 'Windows'
       - name: Run tests
         run: |
           go version
@@ -40,3 +52,4 @@ jobs:
             echo "No Makefile test target, running tests with race detection as default behavior"
             go test -race ./...
           fi
+        shell: bash # to use bash on Windows

Makefile

@@ -6,7 +6,7 @@ run: grol
 	# Interactive debug run: use logger with file and line numbers
 	LOGGER_IGNORE_CLI_MODE=true GOMEMLIMIT=1GiB ./grol -panic -parse -loglevel debug
 
-GEN:=object/type_string.go parser/priority_string.go token/type_string.go
+GEN:=object/type_string.go ast/priority_string.go token/type_string.go
 
 grol: Makefile *.go */*.go $(GEN)
 	CGO_ENABLED=0 go build -trimpath -ldflags="-w -s" -tags "$(GO_BUILD_TAGS)" .
@@ -79,16 +79,13 @@ test: grol
 check: grol
 	./check_samples_double_format.sh examples/*.gr
 
-generate:
-	go generate ./... # if this fails go install golang.org/x/tools/cmd/stringer@latest
-
 generate: $(GEN)
 
 object/type_string.go: object/object.go
-	go generate ./object
+	go generate ./object # if this fails go install golang.org/x/tools/cmd/stringer@latest
 
-parser/priority_string.go: parser/parser.go
-	go generate ./parser
+ast/priority_string.go: ast/ast.go
+	go generate ./ast
 
 token/type_string.go: token/token.go
 	go generate ./token

ast/ast.go

@@ -33,6 +33,7 @@ const (
 )
 
 var Precedences = map[token.Type]Priority{
+	token.DEFINE:     ASSIGN,
 	token.ASSIGN:     ASSIGN,
 	token.OR:         OR,
 	token.AND:        AND,

eval/eval.go

@@ -29,13 +29,15 @@ func (s *State) evalAssignment(right object.Object, node *ast.InfixExpression) o
 		return s.evalIndexAssigment(idxE.Left, index, right)
 	case token.LBRACKET:
 		idxE := node.Left.(*ast.IndexExpression)
-		index := s.evalInternal(idxE.Index)
+		index := s.Eval(idxE.Index)
 		return s.evalIndexAssigment(idxE.Left, index, right)
 	case token.IDENT:
 		id, _ := node.Left.(*ast.Identifier)
 		name := id.Literal()
 		log.LogVf("eval assign %#v to %s", right, name)
-		return s.env.Set(name, right) // Propagate possible error (constant, extension names setting).
+		// Propagate possible error (constant, extension names setting).
+		// Distinguish between define and assign, define (:=) forces a new variable.
+		return s.env.CreateOrSet(name, right, node.Type() == token.DEFINE)
 	default:
 		return s.NewError("assignment to non identifier: " + node.Left.Value().DebugString())
 	}
@@ -50,6 +52,7 @@ func (s *State) evalIndexAssigment(which ast.Node, index, value object.Object) o
 	if !ok {
 		return s.NewError("identifier not found: " + id.Literal())
 	}
+	val = object.Value(val) // deref.
 	switch val.Type() {
 	case object.ARRAY:
 		if index.Type() != object.INTEGER {
@@ -102,13 +105,14 @@ func (s *State) evalPrefixIncrDecr(operator token.Type, node ast.Node) object.Ob
 	log.LogVf("eval prefix %s", ast.DebugString(node))
 	nv := node.Value()
 	if nv.Type() != token.IDENT {
-		return s.NewError("can't increment/decrement " + nv.DebugString())
+		return s.NewError("can't prefix increment/decrement " + nv.DebugString())
 	}
 	id := nv.Literal()
 	val, ok := s.env.Get(id)
 	if !ok {
 		return s.NewError("identifier not found: " + id)
 	}
+	val = object.Value(val) // deref.
 	toAdd := int64(1)
 	if operator == token.DECR {
 		toAdd = -1
@@ -119,7 +123,7 @@ func (s *State) evalPrefixIncrDecr(operator token.Type, node ast.Node) object.Ob
 	case object.Float:
 		return s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)}) // So PI++ fails not silently.
 	default:
-		return s.NewError("can't increment/decrement " + val.Type().String())
+		return s.NewError("can't prefix increment/decrement " + val.Type().String())
 	}
 }
 
@@ -130,6 +134,7 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 	if !ok {
 		return s.NewError("identifier not found: " + id)
 	}
+	val = object.Value(val) // deref.
 	var toAdd int64
 	switch node.Type() {
 	case token.INCR:
@@ -146,7 +151,7 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 	case object.Float:
 		oerr = s.env.Set(id, object.Float{Value: val.Value + float64(toAdd)}) // So PI++ fails not silently.
 	default:
-		return s.NewError("can't increment/decrement " + val.Type().String())
+		return s.NewError("can't postfix increment/decrement " + val.Type().String())
 	}
 	if oerr.Type() == object.ERROR {
 		return oerr
@@ -155,7 +160,9 @@ func (s *State) evalPostfixExpression(node *ast.PostfixExpression) object.Object
 }
 
 // Doesn't unwrap return - return bubbles up.
-func (s *State) evalInternal(node any) object.Object { //nolint:funlen,gocyclo,gocognit // quite a lot of cases.
+// Initially this was the one to use internally recursively, except for when evaluating a function
+// but now it's less clear because of the need to unwrap references too. TODO: fix/clarify.
+func (s *State) evalInternal(node any) object.Object { //nolint:funlen,gocyclo // quite a lot of cases.
 	if s.Context != nil && s.Context.Err() != nil {
 		return s.Error(s.Context.Err())
 	}
@@ -180,7 +187,7 @@ func (s *State) evalInternal(node any) object.Object { //nolint:funlen,gocyclo,g
 		case token.INCR, token.DECR:
 			return s.evalPrefixIncrDecr(node.Type(), node.Right)
 		default:
-			right := s.evalInternal(node.Right)
+			right := s.Eval(node.Right)
 			if right.Type() == object.ERROR {
 				return right
 			}
@@ -191,7 +198,7 @@ func (s *State) evalInternal(node any) object.Object { //nolint:funlen,gocyclo,g
 	case *ast.InfixExpression:
 		log.LogVf("eval infix %s", node.DebugString())
 		// Eval and not evalInternal because we need to unwrap "return".
-		if node.Token.Type() == token.ASSIGN {
+		if node.Token.Type() == token.ASSIGN || node.Token.Type() == token.DEFINE {
 			return s.evalAssignment(s.Eval(node.Right), node)
 		}
 		// Humans expect left to right evaluations.
@@ -256,7 +263,7 @@ func (s *State) evalInternal(node any) object.Object { //nolint:funlen,gocyclo,g
 		}
 		return fn
 	case *ast.CallExpression:
-		f := s.evalInternal(node.Function)
+		f := s.Eval(node.Function)
 		if f.Type() == object.ERROR {
 			return f
 		}
@@ -278,36 +285,48 @@ func (s *State) evalInternal(node any) object.Object { //nolint:funlen,gocyclo,g
 	case *ast.MapLiteral:
 		return s.evalMapLiteral(node)
 	case *ast.IndexExpression:
-		left := s.evalInternal(node.Left)
-		var index object.Object
-		if node.Token.Type() == token.DOT {
-			// index is the string value and not an identifier.
-			index = object.String{Value: node.Index.Value().Literal()}
-		} else {
-			if node.Index.Value().Type() == token.COLON {
-				rangeExp := node.Index.(*ast.InfixExpression)
-				return s.evalIndexRangeExpression(left, rangeExp.Left, rangeExp.Right)
-			}
-			index = s.evalInternal(node.Index)
-		}
-		return s.evalIndexExpression(left, index)
+		return s.evalIndexExpression(s.Eval(node.Left), node)
 	case *ast.Comment:
 		return object.NULL
 	}
 	return s.Errorf("unknown node type: %T", node)
 }
 
+func (s *State) evalIndexExpression(left object.Object, node *ast.IndexExpression) object.Object {
+	if left.Type() == object.ERROR {
+		return left
+	}
+	var index object.Object
+	if node.Token.Type() == token.DOT {
+		// index is the string value and not an identifier to resolve.
+		key := node.Index.Value()
+		if key.Type() != token.STRING && key.Type() != token.IDENT {
+			return s.Errorf("index expression with . not string: %s", key.Literal())
+		}
+		return s.evalIndexExpressionIdx(left, object.String{Value: key.Literal()})
+	}
+	if node.Index.Value().Type() == token.COLON {
+		rangeExp := node.Index.(*ast.InfixExpression)
+		return s.evalIndexRangeExpression(left, rangeExp.Left, rangeExp.Right)
+	}
+	index = s.Eval(node.Index)
+	if index.Type() == object.ERROR {
+		return index
+	}
+	return s.evalIndexExpressionIdx(left, index)
+}
+
 func (s *State) evalMapLiteral(node *ast.MapLiteral) object.Object {
 	result := object.NewMapSize(len(node.Order))
 
 	for _, keyNode := range node.Order {
 		valueNode := node.Pairs[keyNode]
-		key := s.evalInternal(keyNode)
+		key := s.Eval(keyNode)
 		if !object.Equals(key, key) {
 			log.Warnf("key %s is not hashable", key.Inspect())
 			return s.NewError("key " + key.Inspect() + " is not hashable")
 		}
-		value := s.evalInternal(valueNode)
+		value := s.Eval(valueNode)
 		result = result.Set(key, value)
 	}
 	return result
@@ -399,13 +418,13 @@ func (s *State) evalBuiltin(node *ast.Builtin) object.Object {
 }
 
 func (s *State) evalIndexRangeExpression(left object.Object, leftIdx, rightIdx ast.Node) object.Object {
-	leftIndex := s.evalInternal(leftIdx)
+	leftIndex := s.Eval(leftIdx)
 	nilRight := (rightIdx == nil)
 	var rightIndex object.Object
 	if nilRight {
 		log.Debugf("eval index %s[%s:]", left.Inspect(), leftIndex.Inspect())
 	} else {
-		rightIndex = s.evalInternal(rightIdx)
+		rightIndex = s.Eval(rightIdx)
 		log.Debugf("eval index %s[%s:%s]", left.Inspect(), leftIndex.Inspect(), rightIndex.Inspect())
 	}
 	if leftIndex.Type() != object.INTEGER || (!nilRight && rightIndex.Type() != object.INTEGER) {
@@ -445,7 +464,7 @@ func (s *State) evalIndexRangeExpression(left object.Object, leftIdx, rightIdx a
 	}
 }
 
-func (s *State) evalIndexExpression(left, index object.Object) object.Object {
+func (s *State) evalIndexExpressionIdx(left, index object.Object) object.Object {
 	idxOrZero := index
 	if idxOrZero.Type() == object.NIL {
 		idxOrZero = object.Integer{Value: 0}
@@ -473,13 +492,13 @@ func (s *State) evalIndexExpression(left, index object.Object) object.Object {
 	}
 }
 
-func evalMapIndexExpression(hash, key object.Object) object.Object {
-	m := hash.(object.Map)
+func evalMapIndexExpression(assoc, key object.Object) object.Object {
+	m := assoc.(object.Map)
 	v, ok := m.Get(key)
 	if !ok {
 		return object.NULL
 	}
-	return v
+	return v // already unwrapped (index has been Eval'ed)
 }
 
 func evalArrayIndexExpression(array, index object.Object) object.Object {
@@ -573,6 +592,8 @@ func (s *State) applyFunction(name string, fn object.Object, args []object.Objec
 	}
 	if after != before {
 		log.Debugf("Cache miss for %s %v, %d get misses", function.CacheKey, args, after-before)
+		// A miss here is a miss upstack
+		s.env.TriggerNoCache()
 		return res
 	}
 	// Don't cache errors, as it could be due to binding for instance.
@@ -619,22 +640,23 @@ func extendFunctionEnv(
 			name, len(args), atLeast, n)}
 	}
 	for paramIdx, param := range params {
-		oerr := env.Set(param.Value().Literal(), args[paramIdx])
+		// By definition function parameters are local copies, deref argument values:
+		oerr := env.CreateOrSet(param.Value().Literal(), object.Value(args[paramIdx]), true)
 		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.SetNoChecks("..", object.NewArray(extra))
+		env.SetNoChecks("..", object.NewArray(extra), true)
 	}
 	// for recursion in anonymous functions.
-	// TODO: consider not having to keep setting this in the function's env and treating as a keyword.
-	env.SetNoChecks("self", fn)
+	// TODO: consider not having to keep setting this in the function's env and treating as a keyword or magic var like info.
+	env.SetNoChecks("self", fn, true)
 	// 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)
+		env.SetNoChecks(name, fn, true)
 	}
 	return env, nil
 }
@@ -747,7 +769,7 @@ func (s *State) evalForSpecialForms(fe *ast.ForExpression) (object.Object, bool)
 		return s.evalForInteger(fe, &startInt, end.(object.Integer), name), true
 	}
 	// Evaluate:
-	v := s.Eval(ie.Right)
+	v := s.evalInternal(ie.Right)
 	switch v.Type() {
 	case object.INTEGER:
 		return s.evalForInteger(fe, nil, v.(object.Integer), name), true

eval/eval_api.go

@@ -139,7 +139,10 @@ func (s *State) Eval(node any) object.Object {
 		if returnValue.ControlType != token.RETURN {
 			return s.Errorf("unexpected control type %v outside of for loops", returnValue.ControlType)
 		}
-		return returnValue.Value
+		result = returnValue.Value
+	}
+	if refValue, ok := result.(object.Reference); ok {
+		return object.Value(refValue)
 	}
 	return result
 }

eval/eval_test.go

@@ -996,3 +996,45 @@ func TestParenInIf(t *testing.T) {
 		t.Errorf("wrong result, got %q", res.Inspect())
 	}
 }
+
+func TestSelfRef(t *testing.T) {
+	inp := `a=1 ()=>{a=a}()`
+	s := eval.NewState()
+	res, err := eval.EvalString(s, inp, false)
+	if err != nil {
+		t.Errorf("should not have errored: %v", err)
+	}
+	expected := "1"
+	if res.Inspect() != expected {
+		t.Errorf("wrong result, got %q", res.Inspect())
+	}
+}
+
+func TestAliasTwice(t *testing.T) {
+	inp := `a=1; b=2;()=>{a=b}();b=5;()=>{a=b}()` // should not crash
+	s := eval.NewState()
+	res, err := eval.EvalString(s, inp, false)
+	if err != nil {
+		t.Errorf("should not have errored: %v", err)
+	}
+	expected := "5"
+	if res.Inspect() != expected {
+		t.Errorf("wrong result, got %q", res.Inspect())
+	}
+}
+
+func TestIncrMatrix(t *testing.T) {
+	inp := `m={"v":3};()=>{m.v++}();m.v`
+	s := eval.NewState()
+	res, err := eval.EvalString(s, inp, false)
+	if err == nil { // TODO fix https://github.com/grol-io/grol/issues/189
+		// t.Errorf("should not have errored: %v", err)
+		t.Fatalf("should have errored, got %v", res)
+	}
+	// once implement res should be 4.
+	expected := "eval error: <err: index expression with . not string: ++ in ()=>m.v++>"
+	actual := err.Error()
+	if actual != expected {
+		t.Errorf("wrong error, got %q instead of %q", actual, expected)
+	}
+}