Skip to content

Commit

Permalink
xreflect: delete method Type.RemoveMethods().
Browse files Browse the repository at this point in the history
It was used by Universe.FromReflectType() to remove wrapper methods from embedded fields,
but it was unsafe and forgot to update the reflect.Value of Type's methods.
The new solution is not to add at all wrapper methods from embedded fields.

This should hopefully completely fix issue #122
  • Loading branch information
cosmos72 committed Jun 25, 2022
1 parent 1bee429 commit c545040
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 110 deletions.
6 changes: 4 additions & 2 deletions _experiments/issue122.gomacro
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ func getData() *req.Response {
if err != nil {
return nil
}
fmt.Println(len(res.Bytes()))
fmt.Println("Response length:", len(res.Bytes()))
return res
}

res := getData()
res.GetHeader()

fmt.Println("Cookies:", res.Cookies())
fmt.Println("Content-Type:", res.GetHeader("Content-Type"))
7 changes: 1 addition & 6 deletions fast/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,14 +390,9 @@ func (cg *CompGlobals) parseUntyped(untypedstr string) (UntypedLit, xr.Type) {
func (imp *Import) loadTypes(cg *CompGlobals, pkgref *genimport.PackageRef) {
v := cg.Universe
types := make(map[string]xr.Type)
wrappers := pkgref.Wrappers
for name, rtype := range pkgref.Types {
// Universe.FromReflectType uses cached *types.Package if possible
t := v.FromReflectType(rtype)
if twrappers := wrappers[name]; len(twrappers) != 0 {
t.RemoveMethods(twrappers, "")
}
types[name] = t
types[name] = v.FromReflectType(rtype)
}
imp.Types = types
}
Expand Down
6 changes: 3 additions & 3 deletions go/types/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (a *Array) Elem() Type { return a.elem }

// A Slice represents a slice type.
type Slice struct {
elem Type
elem Type
lazymethods []*Func // pre-declared methods Append, Cap, Copy, Get, GetAddr, Len, Set. lazily initialized.
}

Expand Down Expand Up @@ -388,8 +388,8 @@ func (t *Interface) Complete() *Interface {

// A Map represents a map type.
type Map struct {
key, elem Type
lazymethods []*Func // pre-declared methods Delete, Get, Len, Set. lazily initialized.
key, elem Type
lazymethods []*Func // pre-declared methods Delete, Get, Len, Set. lazily initialized.
}

// NewMap returns a new map for the given key and element types.
Expand Down
6 changes: 0 additions & 6 deletions xreflect/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,6 @@ func (t Type) Out(i int) Type {
return t(z{}).Out(i)
}

// RemoveMethods removes given methods from type.
// It panics if the type is unnamed.
func (t Type) RemoveMethods(names []string, pkgpath string) {
t(z{}).RemoveMethods(names, pkgpath)
}

// SetUnderlying sets the underlying type of a named type and marks it as complete.
// It panics if the type is unnamed, or if the underlying type is named,
// or if SetUnderlying() was already invoked on the named type.
Expand Down
61 changes: 29 additions & 32 deletions xreflect/fromreflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
"go/token"
r "reflect"
"strings"
"unsafe"

"github.com/cosmos72/gomacro/go/types"
"github.com/cosmos72/gomacro/imports"
)

// TypeOf creates a Type corresponding to reflect.TypeOf() of given value.
Expand Down Expand Up @@ -244,26 +244,22 @@ func (v *Universe) addmethods(t Type, rtype r.Type) Type {
v.debugf("adding methods to: %v", xt)
defer de(bug(v))
}
if len(xt.methodvalue) < methodN {
methodvalue := make([]r.Value, methodN)
copy(xt.methodvalue, methodvalue)
xt.methodvalue = methodvalue
}
resizemethodvalues(xt, methodN)
if v.rebuild() {
v.RebuildDepth--
}
gtype := xt.gtype.(*types.Named)
cache := makeGmethodMap(gtype)
/*
gotcha := false
if xt.Name() == "Response" && xt.PkgPath() == "github.com/imroc/req/v3" {
gotcha = true
println("gotcha")
}
*/

wrapperMethods := listWrapperMethods(xt)

for i, ni := 0, rtypePtr.NumMethod(); i < ni; i++ {
rmethod := rtypePtr.Method(i)
qname := QName2(rmethod.Name, rmethod.PkgPath)
if isWrapperMethod(wrapperMethods, qname) {
// do NOT add wrapper methods from embedded fields
continue
}
xi, ok := cache[qname]
if ok {
if debug {
Expand Down Expand Up @@ -302,36 +298,37 @@ func (v *Universe) addmethods(t Type, rtype r.Type) Type {
v.debugf("added method[%d->%d] %v // reflect type %v", xi, i, m, m.Type.ReflectType())
}
}
/*
if gotcha {
summaryMethodsOf(xt)
}
*/
return t
}

func summaryMethodsOf(xt *xtype) {
debugf("type %v address = %v", xt, unsafe.Pointer(xt))
debugf("type %v has %d declared methods, and %d total methods", xt, xt.NumExplicitMethod(), xt.NumAllMethod())
gtype, ok := xt.gtype.(*types.Named)
if ok {
debugf("gtype %v has %d declared methods", gtype, gtype.NumMethods())
}
rtypeValue := xt.rtype
rtypePtr := r.PtrTo(rtypeValue)
debugf("rtype %v has %d methods", rtypePtr, rtypePtr.NumMethod())
debugf("rtype %v has %d methods", rtypeValue, rtypeValue.NumMethod())
}

func makeGmethodMap(gtype *types.Named) map[QName]int {
n := gtype.NumMethods()
m := make(map[QName]int)
m := make(map[QName]int, n)
for i := 0; i < n; i++ {
m[QNameGo(gtype.Method(i))] = i
}
return m
}

func listWrapperMethods(xt *xtype) map[string]bool {
pkg, ok := imports.Packages[xt.PkgPath()]
if ok {
names := pkg.Wrappers[xt.Name()]
if len(names) != 0 {
ret := make(map[string]bool)
for _, name := range names {
ret[name] = true
}
return ret
}
}
return nil
}

func isWrapperMethod(wrapperMethods map[string]bool, qname QName) bool {
return len(qname.pkgpath) == 0 && wrapperMethods[qname.name]
}

func (v *Universe) fromReflectField(rfield *r.StructField) StructField {
t := v.fromReflectType(rfield.Type)
name := rfield.Name
Expand Down
6 changes: 2 additions & 4 deletions xreflect/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (t *xtype) method(i int) Method {
checkMethod(t, i)
gfunc := t.gmethod(i)
name := gfunc.Name()
resizemethodvalues(t)
resizemethodvalues(t, t.NumAllMethod())

rtype := t.rtype
var rfunctype r.Type
Expand Down Expand Up @@ -265,7 +265,6 @@ func (t *xtype) makemethod(index int, gfun *types.Func, rfuns *[]r.Value, rfunct
}
if gsig.Recv() != nil {
if nparams+1 != rfunctype.NumIn() {
// summaryMethodsOf(t)
xerrorf(t, `type <%v>: inconsistent %d-th method %q signature:
go/types.Type has receiver <%v> and %d parameters: %v
reflect.Type has %d parameters: %v`, t, index, name, gsig.Recv(), nparams, gsig, rfunctype.NumIn(), rfunctype)
Expand All @@ -291,8 +290,7 @@ func (t *xtype) makemethod(index int, gfun *types.Func, rfuns *[]r.Value, rfunct
}
}

func resizemethodvalues(t *xtype) {
n := t.NumMethod()
func resizemethodvalues(t *xtype, n int) {
if cap(t.methodvalue) < n {
slice := make([]r.Value, n, n+n/2+4)
copy(t.methodvalue, slice)
Expand Down
58 changes: 1 addition & 57 deletions xreflect/named.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ package xreflect
import (
"go/token"
r "reflect"
"sort"
"unsafe"

"github.com/cosmos72/gomacro/go/etoken"

Expand Down Expand Up @@ -158,66 +156,12 @@ func (t *xtype) AddMethod(name string, signature Type) int {
return index
}

// RemoveMethods removes given methods from type.
// It panics if the type is unnamed, or if the signature is not a function type,
func (t *xtype) RemoveMethods(names []string, pkgpath string) {
gtype, ok := t.gtype.(*types.Named)
if !ok {
xerrorf(t, "RemoveMethods on unnamed type %v", t)
}
if len(names) == 0 {
return
}
n1 := gtype.NumMethods()
unsafeRemoveMethods(gtype, names, pkgpath)
n2 := gtype.NumMethods()
if n1 != n2 {
// some existing methods were removed.
// they may be cached in some other type's method cache.
t.universe.InvalidateMethodCache()
}
}

// internal representation of go/types.Named
type unsafeNamed struct {
obj *types.TypeName
underlying types.Type
methods []*types.Func
}

func unsafeRemoveMethods(gtype *types.Named, names []string, pkgpath string) {
names = append([]string{}, names...) // make a copy
sort.Strings(names) // and sort it

gt := (*unsafeNamed)(unsafe.Pointer(gtype))

n1 := len(gt.methods)
n2 := n1
for i, j := 0, 0; i < n1; i++ {
m := gt.methods[i]
name := m.Name()
pos := sort.SearchStrings(names, name)
if pos < len(names) && names[pos] == name && (m.Exported() || m.Pkg().Path() == pkgpath) {
// delete this method
n2--
continue
}
if i != j {
gt.methods[j] = gt.methods[i]
}
j++
}
if n1 != n2 {
gt.methods = gt.methods[:n2]
}
}

// GetMethods returns the pointer to the method values.
// It panics if the type is unnamed
func (t *xtype) GetMethods() *[]r.Value {
if !etoken.GENERICS.V2_CTI() && !t.Named() {
xerrorf(t, "GetMethods on unnamed type %v", t)
}
resizemethodvalues(t)
resizemethodvalues(t, t.NumAllMethod())
return &t.methodvalue
}

0 comments on commit c545040

Please sign in to comment.