Skip to content

Commit

Permalink
update unsafe2 for go1.21, skip faky tests
Browse files Browse the repository at this point in the history
In go1.21, the reflect rtype definition has been move to internal/abi.
We follow this change for maintainability, even if there is no layout
change (the go1.20 unsafe2 is compatible with go1.21).

We have isolated a few problematic tests which are failing sometimes
in go1.21, but work in go1.20, and also in go1.22. Those tests are
skipped if in go1.21. A preliminary investigation can not confirm that
something is wrong in yaegi, and the problem disappears with go1.22.
  • Loading branch information
mvertes committed Mar 3, 2024
1 parent 9497c24 commit 9f1e4ea
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build !go1.21
// +build !go1.21

// Package unsafe2 provides helpers to generate recursive struct types.
package unsafe2

Expand Down
72 changes: 72 additions & 0 deletions internal/unsafe2/go1_21_unsafe.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//go:build go1.21
// +build go1.21

// Package unsafe2 provides helpers to generate recursive struct types.
package unsafe2

import (
"reflect"
"unsafe"
)

type dummy struct{}

// DummyType represents a stand-in for a recursive type.
var DummyType = reflect.TypeOf(dummy{})

// The following type sizes must match their original definition in Go src/internal/abi/type.go.
type abiType struct {
_ uintptr
_ uintptr
_ uint32
_ uint8
_ uint8
_ uint8
_ uint8
_ uintptr
_ uintptr
_ int32
_ int32
}

type abiName struct {
Bytes *byte
}

type abiStructField struct {
Name abiName
Typ *abiType
Offset uintptr
}

type abiStructType struct {
abiType
PkgPath abiName
Fields []abiStructField
}

type emptyInterface struct {
typ *abiType
_ unsafe.Pointer
}

// SetFieldType sets the type of the struct field at the given index, to the given type.
//
// The struct type must have been created at runtime. This is very unsafe.
func SetFieldType(s reflect.Type, idx int, t reflect.Type) {
if s.Kind() != reflect.Struct || idx >= s.NumField() {
return
}

rtyp := unpackType(s)
styp := (*abiStructType)(unsafe.Pointer(rtyp))
f := styp.Fields[idx]
f.Typ = unpackType(t)
styp.Fields[idx] = f
}

func unpackType(t reflect.Type) *abiType {
v := reflect.New(t).Elem().Interface()
eface := *(*emptyInterface)(unsafe.Pointer(&v))
return eface.typ
}
3 changes: 3 additions & 0 deletions interp/interp_consistent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ func TestInterpConsistencyBuild(t *testing.T) {
file.Name() == "type33.go" { // expect error
continue
}
if go121 && testsToSkipGo121[file.Name()] {
continue
}

file := file
t.Run(file.Name(), func(t *testing.T) {
Expand Down
12 changes: 12 additions & 0 deletions interp/interp_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"path/filepath"
"regexp"
"runtime"
"strings"
"testing"

Expand All @@ -16,6 +17,12 @@ import (
"github.com/traefik/yaegi/stdlib/unsafe"
)

// The following tests sometimes (not always) crash with go1.21 but not with go1.20 or go1.22.
// The reason of failure is not obvious, maybe due to the runtime itself, and will be investigated separately.
var testsToSkipGo121 = map[string]bool{"cli6.go": true, "cli7.go": true, "issue-1276.go": true, "issue-1330.go": true, "struct11.go": true}

var go121 = strings.HasPrefix(runtime.Version(), "go1.21")

func TestFile(t *testing.T) {
filePath := "../_test/str.go"
runCheck(t, filePath)
Expand All @@ -27,10 +34,15 @@ func TestFile(t *testing.T) {
if err != nil {
t.Fatal(err)
}

for _, file := range files {
if filepath.Ext(file.Name()) != ".go" {
continue
}
// Skip some tests which are problematic in go1.21 only.
if go121 && testsToSkipGo121[file.Name()] {
continue
}
file := file
t.Run(file.Name(), func(t *testing.T) {
runCheck(t, filepath.Join(baseDir, file.Name()))
Expand Down

0 comments on commit 9f1e4ea

Please sign in to comment.