interp: fix map assignment from arithmetic operations

The logic to trigger assigment optimizations has been refactored for
clarity, and to exclude assignments to map entries.

Fixes #981.

.goreleaser.yml

@@ -47,7 +47,7 @@ archives:
       - LICENSE
 
 brews:
-  - github:
+  - tap:
       owner: traefik
       name: homebrew-tap
     commit_author:

_test/const22.go

@@ -0,0 +1,15 @@
+package main
+
+const (
+	numDec uint8 = (1 << iota) / 2
+	numHex
+	numOct
+	numFloat
+)
+
+func main() {
+	println(13 & (numHex | numOct))
+}
+
+// Output:
+// 1

_test/interface48.go

@@ -0,0 +1,17 @@
+package main
+
+import "fmt"
+
+type I1 interface{ A }
+
+type A = I2
+
+type I2 interface{ F() I1 }
+
+func main() {
+	var i I1
+	fmt.Println(i)
+}
+
+// Output:
+// <nil>

_test/interface49.go

@@ -0,0 +1,45 @@
+package main
+
+type Descriptor interface {
+	ParentFile() FileDescriptor
+}
+
+type FileDescriptor interface {
+	Enums() EnumDescriptors
+	Services() ServiceDescriptors
+}
+
+type EnumDescriptors interface {
+	Get(i int) EnumDescriptor
+}
+
+type EnumDescriptor interface {
+	Values() EnumValueDescriptors
+}
+
+type EnumValueDescriptors interface {
+	Get(i int) EnumValueDescriptor
+}
+
+type EnumValueDescriptor interface {
+	Descriptor
+}
+
+type ServiceDescriptors interface {
+	Get(i int) ServiceDescriptor
+}
+
+type ServiceDescriptor interface {
+	Descriptor
+	isServiceDescriptor
+}
+
+type isServiceDescriptor interface{ ProtoType(ServiceDescriptor) }
+
+func main() {
+	var d Descriptor
+	println(d == nil)
+}
+
+// Output:
+// true

_test/issue-1007.go

@@ -0,0 +1,40 @@
+package main
+
+type TypeA struct {
+	B TypeB
+}
+
+type TypeB struct {
+	C1 *TypeC
+	C2 *TypeC
+}
+
+type TypeC struct {
+	Val string
+	D   *TypeD
+	D2  *TypeD
+}
+
+type TypeD struct {
+	Name string
+}
+
+func build() *TypeA {
+	return &TypeA{
+		B: TypeB{
+			C2: &TypeC{Val: "22"},
+		},
+	}
+}
+
+func Bar(s string) string {
+	a := build()
+	return s + "-" + a.B.C2.Val
+}
+
+func main() {
+	println(Bar("test"))
+}
+
+// Output:
+// test-22

_test/issue-981.go

@@ -0,0 +1,15 @@
+package main
+
+import "fmt"
+
+func main() {
+	dp := make(map[int]int)
+	dp[0] = 1
+	for i := 1; i < 10; i++ {
+		dp[i] = dp[i-1] + dp[i-2]
+	}
+	fmt.Printf("%v\n", dp)
+}
+
+// Output:
+// map[0:1 1:1 2:2 3:3 4:5 5:8 6:13 7:21 8:34 9:55]

internal/cmd/genop/genop.go

@@ -209,7 +209,11 @@ func {{$name}}Const(n *node) {
 		v := constant.BinaryOp(vConstantValue(v0), operator, vConstantValue(v1))
 		n.rval.Set(reflect.ValueOf(v))
 		{{- else}}
+		{{- if $op.Int}}
+		v := constant.BinaryOp(constant.ToInt(vConstantValue(v0)), token.{{tokenFromName $name}}, constant.ToInt(vConstantValue(v1)))
+		{{- else}}
 		v := constant.BinaryOp(vConstantValue(v0), token.{{tokenFromName $name}}, vConstantValue(v1))
+		{{- end}}
 		n.rval.Set(reflect.ValueOf(v))
 		{{- end}}
 	{{- if $op.Str}}
@@ -940,6 +944,7 @@ type Op struct {
 	Complex bool   // true if operator applies to complex
 	Shift   bool   // true if operator is a shift operation
 	Bool    bool   // true if operator applies to bool
+	Int     bool   // true if operator applies to int only
 }
 
 func main() {
@@ -968,17 +973,17 @@ func main() {
 	b := &bytes.Buffer{}
 	data := map[string]interface{}{
 		"Arithmetic": map[string]Op{
-			"add":    {"+", true, true, true, false, false},
-			"sub":    {"-", false, true, true, false, false},
-			"mul":    {"*", false, true, true, false, false},
-			"quo":    {"/", false, true, true, false, false},
-			"rem":    {"%", false, false, false, false, false},
-			"shl":    {"<<", false, false, false, true, false},
-			"shr":    {">>", false, false, false, true, false},
-			"and":    {"&", false, false, false, false, false},
-			"or":     {"|", false, false, false, false, false},
-			"xor":    {"^", false, false, false, false, false},
-			"andNot": {"&^", false, false, false, false, false},
+			"add":    {"+", true, true, true, false, false, false},
+			"sub":    {"-", false, true, true, false, false, false},
+			"mul":    {"*", false, true, true, false, false, false},
+			"quo":    {"/", false, true, true, false, false, false},
+			"rem":    {"%", false, false, false, false, false, true},
+			"shl":    {"<<", false, false, false, true, false, true},
+			"shr":    {">>", false, false, false, true, false, true},
+			"and":    {"&", false, false, false, false, false, true},
+			"or":     {"|", false, false, false, false, false, true},
+			"xor":    {"^", false, false, false, false, false, true},
+			"andNot": {"&^", false, false, false, false, false, true},
 		},
 		"IncDec": map[string]Op{
 			"inc": {Name: "+"},
@@ -996,7 +1001,7 @@ func main() {
 			"not":    {Name: "!", Float: false, Bool: true},
 			"neg":    {Name: "-", Float: true, Bool: false},
 			"pos":    {Name: "+", Float: true, Bool: false},
-			"bitNot": {Name: "^", Float: false, Bool: false},
+			"bitNot": {Name: "^", Float: false, Bool: false, Int: true},
 		},
 	}
 	if err = parse.Execute(b, data); err != nil {

interp/cfg.go

@@ -555,10 +555,21 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
 				n.findex = dest.findex
 				n.level = dest.level
 
-				// Propagate type
-				// TODO: Check that existing destination type matches source type
+				// Propagate type.
+				// TODO: Check that existing destination type matches source type.
+
+				// In the following, we attempt to optimize by skipping the assign
+				// operation and setting the source location directly to the destination
+				// location in the frame.
+				//
 				switch {
-				case n.action == aAssign && isCall(src) && dest.typ.cat != interfaceT && !isMapEntry(dest) && !isRecursiveField(dest):
+				case n.action != aAssign:
+					// Do not optimize assign combined with another operator.
+				case isMapEntry(dest):
+					// Setting a map entry needs an additional step, do not optimize.
+					// As we only write, skip the default useless getIndexMap dest action.
+					dest.gen = nop
+				case isCall(src) && dest.typ.cat != interfaceT && !isRecursiveField(dest):
 					// Call action may perform the assignment directly.
 					n.gen = nop
 					src.level = level
@@ -566,32 +577,25 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
 					if src.typ.untyped && !dest.typ.untyped {
 						src.typ = dest.typ
 					}
-				case n.action == aAssign && src.action == aRecv:
+				case src.action == aRecv:
 					// Assign by reading from a receiving channel.
 					n.gen = nop
-					src.findex = dest.findex // Set recv address to LHS
+					src.findex = dest.findex // Set recv address to LHS.
 					dest.typ = src.typ
-				case n.action == aAssign && src.action == aCompositeLit && !isMapEntry(dest):
+				case src.action == aCompositeLit:
 					if dest.typ.cat == valueT && dest.typ.rtype.Kind() == reflect.Interface {
-						// Skip optimisation for assigned binary interface or map entry
-						// which require and additional operation to set the value
+						// Skip optimisation for assigned interface.
 						break
 					}
 					if dest.action == aGetIndex {
-						// optimization does not work when assigning to a struct field. Maybe we're not
-						// setting the right frame index or something, and we would end up not writing at
-						// the right place. So disabling it for now.
+						// Optimization does not work when assigning to a struct field.
 						break
 					}
-					// Skip the assign operation entirely, the source frame index is set
-					// to destination index, avoiding extra memory alloc and duplication.
 					n.gen = nop
 					src.findex = dest.findex
 					src.level = level
-				case n.action == aAssign && len(n.child) < 4 && !src.rval.IsValid() && isArithmeticAction(src):
+				case len(n.child) < 4 && !src.rval.IsValid() && isArithmeticAction(src):
 					// Optimize single assignments from some arithmetic operations.
-					// Skip the assign operation entirely, the source frame index is set
-					// to destination index, avoiding extra memory alloc and duplication.
 					src.typ = dest.typ
 					src.findex = dest.findex
 					src.level = level
@@ -600,15 +604,14 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
 					// Assign to nil.
 					src.rval = reflect.New(dest.typ.TypeOf()).Elem()
 				}
+
 				n.typ = dest.typ
 				if sym != nil {
 					sym.typ = n.typ
 					sym.recv = src.recv
 				}
 				n.level = level
-				if isMapEntry(dest) {
-					dest.gen = nop // skip getIndexMap
-				}
+
 				if n.anc.kind == constDecl {
 					n.gen = nop
 					n.findex = notInFrame
@@ -740,7 +743,13 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
 			wireChild(n)
 			t := n.child[0].typ
 			switch t.cat {
-			case aliasT, ptrT:
+			case aliasT:
+				if isString(t.val.TypeOf()) {
+					n.typ = sc.getType("byte")
+					break
+				}
+				fallthrough
+			case ptrT:
 				n.typ = t.val
 				if t.val.cat == valueT {
 					n.typ = &itype{cat: valueT, rtype: t.val.rtype.Elem()}

interp/gta.go

@@ -319,7 +319,50 @@ func (interp *Interpreter) gtaRetry(nodes []*node, importPath string) error {
 	}
 
 	if len(revisit) > 0 {
-		return revisit[0].cfgErrorf("constant definition loop")
+		n := revisit[0]
+		if n.kind == typeSpec {
+			if err := definedType(n.typ); err != nil {
+				return err
+			}
+		}
+		return n.cfgErrorf("constant definition loop")
+	}
+	return nil
+}
+
+func definedType(typ *itype) error {
+	if !typ.incomplete {
+		return nil
+	}
+	switch typ.cat {
+	case interfaceT, structT:
+		for _, f := range typ.field {
+			if err := definedType(f.typ); err != nil {
+				return err
+			}
+		}
+	case funcT:
+		for _, t := range typ.arg {
+			if err := definedType(t); err != nil {
+				return err
+			}
+		}
+		for _, t := range typ.ret {
+			if err := definedType(t); err != nil {
+				return err
+			}
+		}
+	case mapT:
+		if err := definedType(typ.key); err != nil {
+			return err
+		}
+		fallthrough
+	case aliasT, arrayT, chanT, chanSendT, chanRecvT, ptrT, variadicT:
+		if err := definedType(typ.val); err != nil {
+			return err
+		}
+	case nilT:
+		return typ.node.cfgErrorf("undefined: %s", typ.node.ident)
 	}
 	return nil
 }

interp/interp_eval_test.go

@@ -457,6 +457,7 @@ func TestEvalSliceExpression(t *testing.T) {
 		{src: `a := []int{0,1,2,3}[1::4]`, err: "1:49: 2nd index required in 3-index slice"},
 		{src: `a := []int{0,1,2,3}[1:3:]`, err: "1:51: 3rd index required in 3-index slice"},
 		{src: `a := []int{0,1,2}[3:1]`, err: "invalid index values, must be low <= high <= max"},
+		{pre: func() { eval(t, i, `type Str = string; var r Str = "truc"`) }, src: `r[1]`, res: "114"},
 	})
 }
 

interp/op.go

@@ -253,7 +253,7 @@ func andConst(n *node) {
 	n.rval = reflect.New(t).Elem()
 	switch {
 	case isConst:
-		v := constant.BinaryOp(vConstantValue(v0), token.AND, vConstantValue(v1))
+		v := constant.BinaryOp(constant.ToInt(vConstantValue(v0)), token.AND, constant.ToInt(vConstantValue(v1)))
 		n.rval.Set(reflect.ValueOf(v))
 	case isUint(t):
 		n.rval.SetUint(vUint(v0) & vUint(v1))
@@ -338,7 +338,7 @@ func andNotConst(n *node) {
 	n.rval = reflect.New(t).Elem()
 	switch {
 	case isConst:
-		v := constant.BinaryOp(vConstantValue(v0), token.AND_NOT, vConstantValue(v1))
+		v := constant.BinaryOp(constant.ToInt(vConstantValue(v0)), token.AND_NOT, constant.ToInt(vConstantValue(v1)))
 		n.rval.Set(reflect.ValueOf(v))
 	case isUint(t):
 		n.rval.SetUint(vUint(v0) &^ vUint(v1))
@@ -564,7 +564,7 @@ func orConst(n *node) {
 	n.rval = reflect.New(t).Elem()
 	switch {
 	case isConst:
-		v := constant.BinaryOp(vConstantValue(v0), token.OR, vConstantValue(v1))
+		v := constant.BinaryOp(constant.ToInt(vConstantValue(v0)), token.OR, constant.ToInt(vConstantValue(v1)))
 		n.rval.Set(reflect.ValueOf(v))
 	case isUint(t):
 		n.rval.SetUint(vUint(v0) | vUint(v1))
@@ -799,7 +799,7 @@ func remConst(n *node) {
 	n.rval = reflect.New(t).Elem()
 	switch {
 	case isConst:
-		v := constant.BinaryOp(vConstantValue(v0), token.REM, vConstantValue(v1))
+		v := constant.BinaryOp(constant.ToInt(vConstantValue(v0)), token.REM, constant.ToInt(vConstantValue(v1)))
 		n.rval.Set(reflect.ValueOf(v))
 	case isUint(t):
 		n.rval.SetUint(vUint(v0) % vUint(v1))
@@ -1195,7 +1195,7 @@ func xorConst(n *node) {
 	n.rval = reflect.New(t).Elem()
 	switch {
 	case isConst:
-		v := constant.BinaryOp(vConstantValue(v0), token.XOR, vConstantValue(v1))
+		v := constant.BinaryOp(constant.ToInt(vConstantValue(v0)), token.XOR, constant.ToInt(vConstantValue(v1)))
 		n.rval.Set(reflect.ValueOf(v))
 	case isUint(t):
 		n.rval.SetUint(vUint(v0) ^ vUint(v1))

interp/type.go

@@ -109,7 +109,7 @@ type itype struct {
 	cat         tcat          // Type category
 	field       []structField // Array of struct fields if structT or interfaceT
 	key         *itype        // Type of key element if MapT or nil
-	val         *itype        // Type of value element if chanT,chanSendT, chanRecvT, mapT, ptrT, aliasT, arrayT or variadicT
+	val         *itype        // Type of value element if chanT, chanSendT, chanRecvT, mapT, ptrT, aliasT, arrayT or variadicT
 	recv        *itype        // Receiver type for funcT or nil
 	arg         []*itype      // Argument types if funcT or nil
 	ret         []*itype      // Return types if funcT or nil
@@ -449,6 +449,9 @@ func nodeType(interp *Interpreter, sc *scope, n *node) (*itype, error) {
 			}
 		}
 		t = sym.typ
+		if t.incomplete && t.cat == aliasT && t.val != nil && t.val.cat != nilT {
+			t.incomplete = false
+		}
 		if t.incomplete && t.node != n {
 			m := t.method
 			if t, err = nodeType(interp, sc, t.node); err != nil {
@@ -879,13 +882,19 @@ func isComplete(t *itype, visited map[string]bool) bool {
 	}
 	name := t.path + "/" + t.name
 	if visited[name] {
-		return !t.incomplete
+		return true
 	}
 	if t.name != "" {
 		visited[name] = true
 	}
 	switch t.cat {
-	case aliasT, arrayT, chanT, chanRecvT, chanSendT, ptrT:
+	case aliasT:
+		if t.val != nil && t.val.cat != nilT {
+			// A type aliased to a partially defined type is considered complete, to allow recursivity.
+			return true
+		}
+		fallthrough
+	case arrayT, chanT, chanRecvT, chanSendT, ptrT:
 		return isComplete(t.val, visited)
 	case funcT:
 		complete := true
@@ -899,6 +908,8 @@ func isComplete(t *itype, visited map[string]bool) bool {
 	case interfaceT, structT:
 		complete := true
 		for _, f := range t.field {
+			// Field implicit type names must be marked as visited, to break false circles.
+			visited[f.typ.path+"/"+f.typ.name] = true
 			complete = complete && isComplete(f.typ, visited)
 		}
 		return complete
@@ -1540,7 +1551,7 @@ func hasRecursiveStruct(t *itype, defined map[string]*itype) bool {
 		defined[typ.path+"/"+typ.name] = typ
 
 		for _, f := range typ.field {
-			if hasRecursiveStruct(f.typ, defined) {
+			if hasRecursiveStruct(f.typ, copyDefined(defined)) {
 				return true
 			}
 		}