From 2248851d7700bee036109be660b5fbaef1d7b095 Mon Sep 17 00:00:00 2001 From: Marc Vertes Date: Thu, 5 May 2022 21:31:10 +0200 Subject: [PATCH] interp: fix creation of binary composite types (#1391) * interp: fix creation of binary composite types Use direct assignment instead of reflect.Value Set method to initialize a binary composite type, as for non binary types. It ensures that a new reflect.Value is stored in the frame instead of modifying a possibly existing one, which can defeat the purpose of initializing variables in a body loop. While there, remove the need to have and use a mutex on types. Fixes #1381. * review: rework a bit the test Co-authored-by: mpl --- _test/issue-1381.go | 58 +++++++++++++++++++++++++++++++++++++++++++++ interp/run.go | 15 ++++++------ interp/type.go | 2 -- 3 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 _test/issue-1381.go diff --git a/_test/issue-1381.go b/_test/issue-1381.go new file mode 100644 index 000000000..be8689212 --- /dev/null +++ b/_test/issue-1381.go @@ -0,0 +1,58 @@ +package main + +import ( + "bytes" + "fmt" +) + +func main() { + var bufPtrOne *bytes.Buffer + var bufPtrTwo *bytes.Buffer + var bufPtrThree *bytes.Buffer + var bufPtrFour *bytes.Buffer + + for i := 0; i < 2; i++ { + bufOne := bytes.Buffer{} + bufTwo := &bytes.Buffer{} + var bufThree bytes.Buffer + bufFour := new(bytes.Buffer) + + if bufPtrOne == nil { + bufPtrOne = &bufOne + } else if bufPtrOne == &bufOne { + fmt.Println("bufOne was not properly redeclared") + } else { + fmt.Println("bufOne is properly redeclared") + } + + if bufPtrTwo == nil { + bufPtrTwo = bufTwo + } else if bufPtrTwo == bufTwo { + fmt.Println("bufTwo was not properly redeclared") + } else { + fmt.Println("bufTwo is properly redeclared") + } + + if bufPtrThree == nil { + bufPtrThree = &bufThree + } else if bufPtrThree == &bufThree { + fmt.Println("bufThree was not properly redeclared") + } else { + fmt.Println("bufThree is properly redeclared") + } + + if bufPtrFour == nil { + bufPtrFour = bufFour + } else if bufPtrFour == bufFour { + fmt.Println("bufFour was not properly redeclared") + } else { + fmt.Println("bufFour is properly redeclared") + } + } +} + +// Output: +// bufOne is properly redeclared +// bufTwo is properly redeclared +// bufThree is properly redeclared +// bufFour is properly redeclared diff --git a/interp/run.go b/interp/run.go index e165f405a..c86b68a5a 100644 --- a/interp/run.go +++ b/interp/run.go @@ -9,7 +9,6 @@ import ( "reflect" "regexp" "strings" - "sync" ) // bltn type defines functions which run at CFG execution. @@ -2626,6 +2625,9 @@ func doCompositeBinStruct(n *node, hasType bool) { } } + frameIndex := n.findex + l := n.level + n.exec = func(f *frame) bltn { s := reflect.New(typ).Elem() for i, v := range values { @@ -2636,7 +2638,7 @@ func doCompositeBinStruct(n *node, hasType bool) { case d.Kind() == reflect.Ptr: d.Set(s.Addr()) default: - d.Set(s) + getFrame(f, l).data[frameIndex] = s } return next } @@ -2661,8 +2663,6 @@ func doComposite(n *node, hasType bool, keyed bool) { if typ.cat == ptrT || typ.cat == aliasT { typ = typ.val } - var mu sync.Mutex - typ.mu = &mu child := n.child if hasType { child = n.child[1:] @@ -2701,11 +2701,10 @@ func doComposite(n *node, hasType bool, keyed bool) { frameIndex := n.findex l := n.level + rt := typ.TypeOf() + n.exec = func(f *frame) bltn { - typ.mu.Lock() - // No need to call zero() as doComposite is only called for a structT. - a := reflect.New(typ.TypeOf()).Elem() - typ.mu.Unlock() + a := reflect.New(rt).Elem() for i, v := range values { a.Field(i).Set(v(f)) } diff --git a/interp/type.go b/interp/type.go index f98127011..c6955d41a 100644 --- a/interp/type.go +++ b/interp/type.go @@ -7,7 +7,6 @@ import ( "reflect" "strconv" "strings" - "sync" "github.com/traefik/yaegi/internal/unsafe2" ) @@ -110,7 +109,6 @@ type structField struct { // itype defines the internal representation of types in the interpreter. type itype struct { - mu *sync.Mutex cat tcat // Type category field []structField // Array of struct fields if structT or interfaceT key *itype // Type of key element if MapT or nil