Skip to content

Commit

Permalink
interp: fix creation of binary composite types (#1391)
Browse files Browse the repository at this point in the history
* 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 <mathieu.lonjaret@gmail.com>
  • Loading branch information
mvertes and mpl authored May 5, 2022
1 parent f74d1ea commit 2248851
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 10 deletions.
58 changes: 58 additions & 0 deletions _test/issue-1381.go
Original file line number Diff line number Diff line change
@@ -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
15 changes: 7 additions & 8 deletions interp/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"reflect"
"regexp"
"strings"
"sync"
)

// bltn type defines functions which run at CFG execution.
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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:]
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 0 additions & 2 deletions interp/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"reflect"
"strconv"
"strings"
"sync"

"github.com/traefik/yaegi/internal/unsafe2"
)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2248851

Please sign in to comment.