closure tests passing

gnovm/pkg/gnolang/op_assign.go

@@ -10,8 +10,11 @@ func (m *Machine) doOpDefine() {
 	for i := 0; i < len(s.Lhs); i++ {
 		// Get name and value of i'th term.
 		nx := s.Lhs[i].(*NameExpr)
+
 		// Finally, define (or assign if loop block).
 		ptr := lb.GetPointerTo(m.Store, nx.Path)
+		m.handleRedefinition(lb, nx.Path, ptr.TV)
+
 		// XXX HACK (until value persistence impl'd)
 		if m.ReadOnly {
 			if oo, ok := ptr.Base.(Object); ok {
@@ -24,6 +27,35 @@ func (m *Machine) doOpDefine() {
 	}
 }
 
+// handleRedefinition handles a special case where a named symbol is redefined within
+// the same block. This can happen in a loop block or a combination of labels and gotos.
+// During redefinitions, we should check if there are closures from within this block referencing
+// this symbol. If so, publish the value to the function value before it is redefined with a new value.
+func (m *Machine) handleRedefinition(block *Block, valuePath ValuePath, tv *TypedValue) {
+	// This is only a redifinition if the type is not nil. Also don't bother with
+	// the special case of the blank identifier; the index of this identifier can only
+	// have unintended side effects on actual zero indexed identifiers.
+	if tv.T == nil || valuePath.Name == "_" {
+		return
+	}
+
+	// Do nothing if the block has no closure references.
+	var funcRefs []*funcValueWithDepth
+	if funcRefs = block.closureRefs[int(valuePath.Index)]; funcRefs == nil {
+		return
+	}
+
+	tvCopy := tv.unrefCopy(m.Alloc, m.Store)
+	for _, ref := range funcRefs {
+		newValuePath := valuePath
+		newValuePath.Depth = ref.depth
+		ref.fv.finalizedNamedTypes[newValuePath] = &tvCopy
+	}
+
+	// This particular reference has been resolved.
+	delete(block.closureRefs, int(valuePath.Index))
+}
+
 func (m *Machine) doOpAssign() {
 	s := m.PopStmt().(*AssignStmt)
 	// Assign each value evaluated for Lhs.

gnovm/pkg/gnolang/op_call.go

@@ -57,6 +57,8 @@ func (m *Machine) doOpCall() {
 	// Create new block scope.
 	clo := fr.Func.GetClosure(m.Store)
 	b := m.Alloc.NewBlock(fr.Func.GetSource(m.Store), clo)
+	b.resolvedClosureValues = fv.finalizedNamedTypes
+
 	m.PushBlock(b)
 	if fv.nativeBody == nil && fv.NativePkg != "" {
 		// native function, unmarshaled so doesn't have nativeBody yet

gnovm/pkg/gnolang/op_decl.go

@@ -60,6 +60,7 @@ func (m *Machine) doOpValueDecl() {
 		}
 		nx := s.NameExprs[i]
 		ptr := lb.GetPointerTo(m.Store, nx.Path)
+		m.handleRedefinition(lb, nx.Path, ptr.TV)
 		ptr.Assign2(m.Alloc, m.Store, m.Realm, tv, false)
 	}
 }

gnovm/pkg/gnolang/op_exec.go

@@ -131,6 +131,21 @@ func (m *Machine) doOpExec(op Op) {
 				// or uh...
 				bs.Active = next
 				s = next
+
+				// There is a `bs.Post` statement. This may mutate the value of the loop counter.
+				// Push the value now to potential closures before the post statement is executed.
+				// if block := m.LastBlock(); block.resolvedClosureValues[s.]
+				if block := m.LastBlock(); len(block.closureRefs) != 0 {
+					if assignStmt, ok := block.Source.(*ForStmt).Init.(*AssignStmt); ok {
+						for _, expr := range assignStmt.Lhs {
+							if nameExpr, ok := expr.(*NameExpr); ok {
+								// Is this okay? the ptr's TV type will still not be nil in this case?
+								ptr := block.GetPointerTo(m.Store, nameExpr.Path)
+								m.handleRedefinition(m.LastBlock(), nameExpr.Path, ptr.TV)
+							}
+						}
+					}
+				}
 				goto EXEC_SWITCH
 			}
 		} else {
@@ -173,6 +188,7 @@ func (m *Machine) doOpExec(op Op) {
 				case DEFINE:
 					knxp := bs.Key.(*NameExpr).Path
 					ptr := m.LastBlock().GetPointerTo(m.Store, knxp)
+					m.handleRedefinition(m.LastBlock(), knxp, ptr.TV)
 					ptr.TV.Assign(m.Alloc, iv, false)
 				default:
 					panic("should not happen")
@@ -188,6 +204,7 @@ func (m *Machine) doOpExec(op Op) {
 				case DEFINE:
 					vnxp := bs.Value.(*NameExpr).Path
 					ptr := m.LastBlock().GetPointerTo(m.Store, vnxp)
+					m.handleRedefinition(m.LastBlock(), vnxp, ptr.TV)
 					ptr.TV.Assign(m.Alloc, ev, false)
 				default:
 					panic("should not happen")
@@ -269,6 +286,7 @@ func (m *Machine) doOpExec(op Op) {
 				case DEFINE:
 					knxp := bs.Key.(*NameExpr).Path
 					ptr := m.LastBlock().GetPointerTo(m.Store, knxp)
+					m.handleRedefinition(m.LastBlock(), knxp, ptr.TV)
 					ptr.TV.Assign(m.Alloc, iv, false)
 				default:
 					panic("should not happen")
@@ -282,6 +300,7 @@ func (m *Machine) doOpExec(op Op) {
 				case DEFINE:
 					vnxp := bs.Value.(*NameExpr).Path
 					ptr := m.LastBlock().GetPointerTo(m.Store, vnxp)
+					m.handleRedefinition(m.LastBlock(), vnxp, ptr.TV)
 					ptr.TV.Assign(m.Alloc, ev, false)
 				default:
 					panic("should not happen")
@@ -362,6 +381,7 @@ func (m *Machine) doOpExec(op Op) {
 				case DEFINE:
 					knxp := bs.Key.(*NameExpr).Path
 					ptr := m.LastBlock().GetPointerTo(m.Store, knxp)
+					m.handleRedefinition(m.LastBlock(), knxp, ptr.TV)
 					ptr.TV.Assign(m.Alloc, kv, false)
 				default:
 					panic("should not happen")
@@ -375,6 +395,7 @@ func (m *Machine) doOpExec(op Op) {
 				case DEFINE:
 					vnxp := bs.Value.(*NameExpr).Path
 					ptr := m.LastBlock().GetPointerTo(m.Store, vnxp)
+					m.handleRedefinition(m.LastBlock(), vnxp, ptr.TV)
 					ptr.TV.Assign(m.Alloc, vv, false)
 				default:
 					panic("should not happen")

gnovm/pkg/gnolang/op_expressions.go

@@ -3,6 +3,7 @@ package gnolang
 import (
 	"fmt"
 	"reflect"
+	"sort"
 )
 
 // OpBinary1 defined in op_binary.go
@@ -681,20 +682,86 @@ func (m *Machine) doOpFuncLit() {
 	x := m.PopExpr().(*FuncLitExpr)
 	ft := m.PopValue().V.(TypeValue).Type.(*FuncType)
 	lb := m.LastBlock()
+
+	// Assign add references to this func to the blocks
+	// that contain variables used by this func. This will
+	// enable the blocks to publish the finalized values for this
+	// func to use in the case where they are redefined as in a
+	// loop or label/goto instance.
+	externNames := x.GetExternNames()
+	nameExpressions := make([]*NameExpr, 0, len(externNames))
+	for _, n := range externNames {
+		vp := x.GetPathForName(m.Store, n)
+		if vp.Depth == 0 { // skip uverse name
+			continue
+		}
+
+		nameExpressions = append(
+			nameExpressions,
+			&NameExpr{
+				Name: n,
+				Path: vp,
+			},
+		)
+	}
+
+	// Sort by path depth so we can traverse the named expressions as we bubble up
+	// through the parent blocks.
+	sort.Slice(
+		nameExpressions,
+		func(i, j int) bool {
+			return nameExpressions[i].Path.Depth < nameExpressions[j].Path.Depth
+		},
+	)
+
 	m.Alloc.AllocateFunc()
-	m.PushValue(TypedValue{
-		T: ft,
-		V: &FuncValue{
-			Type:       ft,
-			IsMethod:   false,
-			Source:     x,
-			Name:       "",
-			Closure:    lb,
-			PkgPath:    m.Package.PkgPath,
-			body:       x.Body,
-			nativeBody: nil,
+	funcValue := &FuncValue{
+		Type:                ft,
+		IsMethod:            false,
+		Source:              x,
+		Name:                "",
+		Closure:             lb,
+		PkgPath:             m.Package.PkgPath,
+		body:                x.Body,
+		nativeBody:          nil,
+		finalizedNamedTypes: make(map[ValuePath]*TypedValue),
+	}
+
+	scopedBlock := lb
+	lastDepth := uint8(1)
+	for _, expr := range nameExpressions {
+		if expr.Path.Depth-1 != uint8(lastDepth) {
+			for i := uint8(1); i < expr.Path.Depth-1; i++ {
+				scopedBlock = lb.GetParent(m.Store)
+			}
+
+			lastDepth = expr.Path.Depth - 1
+		}
+
+		// Create references back to this function in the blocks where the
+		// name expression values live.
+		indexFuncVals := scopedBlock.closureRefs[int(expr.Path.Index)]
+		indexFuncVals = append(
+			indexFuncVals,
+			&funcValueWithDepth{
+				fv:    funcValue,
+				depth: expr.Path.Depth,
+			},
+		)
+
+		if scopedBlock.closureRefs == nil {
+			scopedBlock.closureRefs = make(map[int][]*funcValueWithDepth)
+		}
+
+		scopedBlock.closureRefs[int(expr.Path.Index)] = indexFuncVals
+	}
+
+	m.PushValue(
+		TypedValue{
+			T: ft,
+			V: funcValue,
 		},
-	})
+	)
 }
 
 func (m *Machine) doOpConvert() {

gnovm/pkg/gnolang/values.go

@@ -537,8 +537,9 @@ type FuncValue struct {
 	NativePkg  string // for native bindings through NativeStore
 	NativeName Name   // not redundant with Name; this cannot be changed in userspace
 
-	body       []Stmt         // function body
-	nativeBody func(*Machine) // alternative to Body
+	body                []Stmt         // function body
+	nativeBody          func(*Machine) // alternative to Body
+	finalizedNamedTypes map[ValuePath]*TypedValue
 }
 
 func (fv *FuncValue) IsNative() bool {
@@ -1023,6 +1024,8 @@ func (tv TypedValue) Copy(alloc *Allocator) (cp TypedValue) {
 		cp.V = cv.Copy(alloc)
 	default:
 		cp = tv
+		cp.N = [8]byte{}
+		copy(cp.N[:], tv.N[:])
 	}
 	return
 }
@@ -2259,6 +2262,16 @@ type Block struct {
 	Parent     Value
 	Blank      TypedValue // captures "_" // XXX remove and replace with global instance.
 	bodyStmt   bodyStmt   // XXX expose for persistence, not needed for MVP.
+
+	// closureRefs maps `Values` indexes to func values. This is used to publish finalized
+	// values to function that reference them when the named values are redefined.
+	closureRefs           map[int][]*funcValueWithDepth
+	resolvedClosureValues map[ValuePath]*TypedValue
+}
+
+type funcValueWithDepth struct {
+	fv    *FuncValue
+	depth uint8
 }
 
 // NOTE: for allocation, use *Allocator.NewBlock.
@@ -2268,9 +2281,10 @@ func NewBlock(source BlockNode, parent *Block) *Block {
 		values = make([]TypedValue, source.GetNumNames())
 	}
 	return &Block{
-		Source: source,
-		Values: values,
-		Parent: parent,
+		Source:      source,
+		Values:      values,
+		Parent:      parent,
+		closureRefs: make(map[int][]*funcValueWithDepth),
 	}
 }
 
@@ -2340,6 +2354,22 @@ func (b *Block) GetPointerToInt(store Store, index int) PointerValue {
 	}
 }
 
+func (b *Block) GetResolvedClosureValuePointer(valuePath ValuePath, offset uint8) (pv PointerValue, found bool) {
+	if b.resolvedClosureValues == nil {
+		return
+	}
+
+	tv, ok := b.resolvedClosureValues[valuePath]
+	if !ok {
+		return
+	}
+
+	return PointerValue{
+		TV:   tv,
+		Base: b,
+	}, true
+}
+
 func (b *Block) GetPointerTo(store Store, path ValuePath) PointerValue {
 	if path.IsBlockBlankPath() {
 		if debug {
@@ -2355,13 +2385,46 @@ func (b *Block) GetPointerTo(store Store, path ValuePath) PointerValue {
 			Index: PointerIndexBlockBlank, // -1
 		}
 	}
+
+	var (
+		funcBlock        *Block
+		closureValuePath ValuePath
+	)
+
 	// NOTE: For most block paths, Depth starts at 1, but
 	// the generation for uverse is 0.  If path.Depth is
 	// 0, it implies that b == uverse, and the condition
 	// would fail as if it were 1.
 	for i := uint8(1); i < path.Depth; i++ {
+		if funcBlock == nil && len(b.resolvedClosureValues) != 0 {
+			// fmt.Println(
+			// 	len(b.resolvedClosureValues),
+			// 	path.Depth,
+			// 	b.resolvedClosureValues,
+			// )
+
+			closureValuePath = path
+			closureValuePath.Depth = closureValuePath.Depth - i + 1
+			if b.resolvedClosureValues[closureValuePath] != nil {
+				funcBlock = b
+			}
+		}
+
 		b = b.GetParent(store)
 	}
+
+	// If this value is being retrieved as part of a closure execution, then try to get
+	// the pointer value from the resolved closure values that were published during a
+	// variable redeifinition in a parent block.
+	if funcBlock != nil {
+		if pointerValue, ok := funcBlock.GetResolvedClosureValuePointer(
+			closureValuePath,
+			path.Depth,
+		); ok {
+			return pointerValue
+		}
+	}
+
 	return b.GetPointerToInt(store, int(path.Index))
 }