cmd/compile: split exported/non-exported methods for interface type

Currently, mhdr/methods is emitted with the same len/cap. There's no way to distinguish between exported and non-exported methods statically. This CL splits mhdr/methods into two parts, use "len" for number of exported methods, and "cap" for all methods. This fixes the bug in issue #22075, which intends to return the number of exported methods but currently return all methods. Note that with this encoding, we still can access either all/exported-only/non-exported-only methods: mhdr[:cap(mhdr)] // all methods mhdr // exported methods mhdr[len(mhdr):cap(mhdr)] // non-exported methods Thank to Matthew Dempsky (@mdempsky) for suggesting this encoding. Fixes #22075 Change-Id: If662adb03ccff27407d55a5578a0ed05a15e7cdd Reviewed-on: https://go-review.googlesource.com/c/go/+/259237 Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
golang · Oct 9, 2020 · 8f26b57 · 8f26b57
1 parent f8df205
commit 8f26b57
Show file tree

Hide file tree

Showing 11 changed files with 124 additions and 64 deletions.
diff --git a/doc/go1.16.html b/doc/go1.16.html
@@ -213,6 +213,14 @@ <h3 id="net"><a href="/pkg/net/">net</a></h3>
   with <code>"use of closed network connection"</code>.
 </p>
 
+<h3 id="reflect"><a href="/pkg/reflect/">reflect</a></h3>
+
+<p><!-- CL 259237, golang.org/issue/22075 -->
+  For interface types and values, <a href="/pkg/reflect/#Value.Method">Method</a>,
+  <a href="/pkg/reflect/#Value.MethodByName">MethodByName</a>, and
+  <a href="/pkg/reflect/#Value.NumMethod">NumMethod</a> now
+  operate on the interface's exported method set, rather than its full method set.
+</p>
 
 <h3 id="text/template/parse"><a href="/pkg/text/template/parse/">text/template/parse</a></h3>
 

diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go
@@ -1275,8 +1275,9 @@ func dtypesym(t *types.Type) *obj.LSym {
 		}
 		ot = dgopkgpath(lsym, ot, tpkg)
 
+		xcount := sort.Search(n, func(i int) bool { return !types.IsExported(m[i].name.Name) })
 		ot = dsymptr(lsym, ot, lsym, ot+3*Widthptr+uncommonSize(t))
-		ot = duintptr(lsym, ot, uint64(n))
+		ot = duintptr(lsym, ot, uint64(xcount))
 		ot = duintptr(lsym, ot, uint64(n))
 		dataAdd := imethodSize() * n
 		ot = dextratype(lsym, ot, t, dataAdd)

diff --git a/src/internal/reflectlite/type.go b/src/internal/reflectlite/type.go
@@ -234,10 +234,13 @@ type imethod struct {
 // interfaceType represents an interface type.
 type interfaceType struct {
 	rtype
-	pkgPath name      // import path
-	methods []imethod // sorted by hash
+	pkgPath    name      // import path
+	expMethods []imethod // sorted by name, see runtime/type.go:interfacetype to see how it is encoded.
 }
 
+func (t *interfaceType) methods() []imethod { return t.expMethods[:cap(t.expMethods)] }
+func (t *interfaceType) isEmpty() bool      { return cap(t.expMethods) == 0 }
+
 // mapType represents a map type.
 type mapType struct {
 	rtype
@@ -695,7 +698,7 @@ func add(p unsafe.Pointer, x uintptr, whySafe string) unsafe.Pointer {
 }
 
 // NumMethod returns the number of interface methods in the type's method set.
-func (t *interfaceType) NumMethod() int { return len(t.methods) }
+func (t *interfaceType) NumMethod() int { return len(t.expMethods) }
 
 // TypeOf returns the reflection Type that represents the dynamic type of i.
 // If i is a nil interface value, TypeOf returns nil.
@@ -732,9 +735,10 @@ func implements(T, V *rtype) bool {
 		return false
 	}
 	t := (*interfaceType)(unsafe.Pointer(T))
-	if len(t.methods) == 0 {
+	if t.isEmpty() {
 		return true
 	}
+	tmethods := t.methods()
 
 	// The same algorithm applies in both cases, but the
 	// method tables for an interface type and a concrete type
@@ -751,10 +755,11 @@ func implements(T, V *rtype) bool {
 	if V.Kind() == Interface {
 		v := (*interfaceType)(unsafe.Pointer(V))
 		i := 0
-		for j := 0; j < len(v.methods); j++ {
-			tm := &t.methods[i]
+		vmethods := v.methods()
+		for j := 0; j < len(vmethods); j++ {
+			tm := &tmethods[i]
 			tmName := t.nameOff(tm.name)
-			vm := &v.methods[j]
+			vm := &vmethods[j]
 			vmName := V.nameOff(vm.name)
 			if vmName.name() == tmName.name() && V.typeOff(vm.typ) == t.typeOff(tm.typ) {
 				if !tmName.isExported() {
@@ -770,7 +775,7 @@ func implements(T, V *rtype) bool {
 						continue
 					}
 				}
-				if i++; i >= len(t.methods) {
+				if i++; i >= len(tmethods) {
 					return true
 				}
 			}
@@ -785,7 +790,7 @@ func implements(T, V *rtype) bool {
 	i := 0
 	vmethods := v.methods()
 	for j := 0; j < int(v.mcount); j++ {
-		tm := &t.methods[i]
+		tm := &tmethods[i]
 		tmName := t.nameOff(tm.name)
 		vm := vmethods[j]
 		vmName := V.nameOff(vm.name)
@@ -803,7 +808,7 @@ func implements(T, V *rtype) bool {
 					continue
 				}
 			}
-			if i++; i >= len(t.methods) {
+			if i++; i >= len(tmethods) {
 				return true
 			}
 		}
@@ -897,7 +902,7 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool {
 	case Interface:
 		t := (*interfaceType)(unsafe.Pointer(T))
 		v := (*interfaceType)(unsafe.Pointer(V))
-		if len(t.methods) == 0 && len(v.methods) == 0 {
+		if t.isEmpty() && v.isEmpty() {
 			return true
 		}
 		// Might have the same methods but still
@@ -962,3 +967,11 @@ func toType(t *rtype) Type {
 func ifaceIndir(t *rtype) bool {
 	return t.kind&kindDirectIface == 0
 }
+
+func isEmptyIface(t *rtype) bool {
+	if t.Kind() != Interface {
+		return false
+	}
+	tt := (*interfaceType)(unsafe.Pointer(t))
+	return tt.isEmpty()
+}
diff --git a/src/internal/reflectlite/value.go b/src/internal/reflectlite/value.go
@@ -228,7 +228,7 @@ func (v Value) Elem() Value {
 	switch k {
 	case Interface:
 		var eface interface{}
-		if v.typ.NumMethod() == 0 {
+		if isEmptyIface(v.typ) {
 			eface = *(*interface{})(v.ptr)
 		} else {
 			eface = (interface{})(*(*interface {
@@ -433,7 +433,7 @@ func (v Value) assignTo(context string, dst *rtype, target unsafe.Pointer) Value
 			return Value{dst, nil, flag(Interface)}
 		}
 		x := valueInterface(v)
-		if dst.NumMethod() == 0 {
+		if isEmptyIface(dst) {
 			*(*interface{})(target) = x
 		} else {
 			ifaceE2I(dst, x, target)

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
@@ -2995,6 +2995,14 @@ func TestUnexportedMethods(t *testing.T) {
 	if got := typ.NumMethod(); got != 0 {
 		t.Errorf("NumMethod=%d, want 0 satisfied methods", got)
 	}
+
+	var i unexpI
+	if got := TypeOf(&i).Elem().NumMethod(); got != 0 {
+		t.Errorf("NumMethod=%d, want 0 satisfied methods", got)
+	}
+	if got := ValueOf(&i).Elem().NumMethod(); got != 0 {
+		t.Errorf("NumMethod=%d, want 0 satisfied methods", got)
+	}
 }
 
 type InnerInt struct {
@@ -3648,21 +3656,21 @@ func TestCallPanic(t *testing.T) {
 	v := ValueOf(T{i, i, i, i, T2{i, i}, i, i, T2{i, i}})
 	badCall(func() { call(v.Field(0).Method(0)) })          // .t0.W
 	badCall(func() { call(v.Field(0).Elem().Method(0)) })   // .t0.W
-	badCall(func() { call(v.Field(0).Method(1)) })          // .t0.w
+	badMethod(func() { call(v.Field(0).Method(1)) })        // .t0.w
 	badMethod(func() { call(v.Field(0).Elem().Method(2)) }) // .t0.w
 	ok(func() { call(v.Field(1).Method(0)) })               // .T1.Y
 	ok(func() { call(v.Field(1).Elem().Method(0)) })        // .T1.Y
-	badCall(func() { call(v.Field(1).Method(1)) })          // .T1.y
+	badMethod(func() { call(v.Field(1).Method(1)) })        // .T1.y
 	badMethod(func() { call(v.Field(1).Elem().Method(2)) }) // .T1.y
 
 	ok(func() { call(v.Field(2).Method(0)) })               // .NamedT0.W
 	ok(func() { call(v.Field(2).Elem().Method(0)) })        // .NamedT0.W
-	badCall(func() { call(v.Field(2).Method(1)) })          // .NamedT0.w
+	badMethod(func() { call(v.Field(2).Method(1)) })        // .NamedT0.w
 	badMethod(func() { call(v.Field(2).Elem().Method(2)) }) // .NamedT0.w
 
 	ok(func() { call(v.Field(3).Method(0)) })               // .NamedT1.Y
 	ok(func() { call(v.Field(3).Elem().Method(0)) })        // .NamedT1.Y
-	badCall(func() { call(v.Field(3).Method(1)) })          // .NamedT1.y
+	badMethod(func() { call(v.Field(3).Method(1)) })        // .NamedT1.y
 	badMethod(func() { call(v.Field(3).Elem().Method(3)) }) // .NamedT1.y
 
 	ok(func() { call(v.Field(4).Field(0).Method(0)) })             // .NamedT2.T1.Y
@@ -3672,7 +3680,7 @@ func TestCallPanic(t *testing.T) {
 
 	badCall(func() { call(v.Field(5).Method(0)) })          // .namedT0.W
 	badCall(func() { call(v.Field(5).Elem().Method(0)) })   // .namedT0.W
-	badCall(func() { call(v.Field(5).Method(1)) })          // .namedT0.w
+	badMethod(func() { call(v.Field(5).Method(1)) })        // .namedT0.w
 	badMethod(func() { call(v.Field(5).Elem().Method(2)) }) // .namedT0.w
 
 	badCall(func() { call(v.Field(6).Method(0)) })        // .namedT1.Y

diff --git a/src/reflect/type.go b/src/reflect/type.go
@@ -386,10 +386,14 @@ type imethod struct {
 // interfaceType represents an interface type.
 type interfaceType struct {
 	rtype
-	pkgPath name      // import path
-	methods []imethod // sorted by hash
+	pkgPath    name      // import path
+	expMethods []imethod // sorted by name, see runtime/type.go:interfacetype to see how it is encoded.
 }
 
+// methods returns t's full method set, both exported and non-exported.
+func (t *interfaceType) methods() []imethod { return t.expMethods[:cap(t.expMethods)] }
+func (t *interfaceType) isEmpty() bool      { return cap(t.expMethods) == 0 }
+
 // mapType represents a map type.
 type mapType struct {
 	rtype
@@ -1049,34 +1053,31 @@ func (d ChanDir) String() string {
 
 // Method returns the i'th method in the type's method set.
 func (t *interfaceType) Method(i int) (m Method) {
-	if i < 0 || i >= len(t.methods) {
-		return
+	if i < 0 || i >= len(t.expMethods) {
+		panic("reflect: Method index out of range")
 	}
-	p := &t.methods[i]
+	p := &t.expMethods[i]
 	pname := t.nameOff(p.name)
 	m.Name = pname.name()
 	if !pname.isExported() {
-		m.PkgPath = pname.pkgPath()
-		if m.PkgPath == "" {
-			m.PkgPath = t.pkgPath.name()
-		}
+		panic("reflect: unexported method: " + pname.name())
 	}
 	m.Type = toType(t.typeOff(p.typ))
 	m.Index = i
 	return
 }
 
-// NumMethod returns the number of interface methods in the type's method set.
-func (t *interfaceType) NumMethod() int { return len(t.methods) }
+// NumMethod returns the number of exported interface methods in the type's method set.
+func (t *interfaceType) NumMethod() int { return len(t.expMethods) }
 
 // MethodByName method with the given name in the type's method set.
 func (t *interfaceType) MethodByName(name string) (m Method, ok bool) {
 	if t == nil {
 		return
 	}
 	var p *imethod
-	for i := range t.methods {
-		p = &t.methods[i]
+	for i := range t.expMethods {
+		p = &t.expMethods[i]
 		if t.nameOff(p.name).name() == name {
 			return t.Method(i), true
 		}
@@ -1485,9 +1486,10 @@ func implements(T, V *rtype) bool {
 		return false
 	}
 	t := (*interfaceType)(unsafe.Pointer(T))
-	if len(t.methods) == 0 {
+	if t.isEmpty() {
 		return true
 	}
+	tmethods := t.methods()
 
 	// The same algorithm applies in both cases, but the
 	// method tables for an interface type and a concrete type
@@ -1504,10 +1506,11 @@ func implements(T, V *rtype) bool {
 	if V.Kind() == Interface {
 		v := (*interfaceType)(unsafe.Pointer(V))
 		i := 0
-		for j := 0; j < len(v.methods); j++ {
-			tm := &t.methods[i]
+		vmethods := v.methods()
+		for j := 0; j < len(vmethods); j++ {
+			tm := &tmethods[i]
 			tmName := t.nameOff(tm.name)
-			vm := &v.methods[j]
+			vm := &vmethods[j]
 			vmName := V.nameOff(vm.name)
 			if vmName.name() == tmName.name() && V.typeOff(vm.typ) == t.typeOff(tm.typ) {
 				if !tmName.isExported() {
@@ -1523,7 +1526,7 @@ func implements(T, V *rtype) bool {
 						continue
 					}
 				}
-				if i++; i >= len(t.methods) {
+				if i++; i >= len(tmethods) {
 					return true
 				}
 			}
@@ -1538,7 +1541,7 @@ func implements(T, V *rtype) bool {
 	i := 0
 	vmethods := v.methods()
 	for j := 0; j < int(v.mcount); j++ {
-		tm := &t.methods[i]
+		tm := &tmethods[i]
 		tmName := t.nameOff(tm.name)
 		vm := vmethods[j]
 		vmName := V.nameOff(vm.name)
@@ -1556,7 +1559,7 @@ func implements(T, V *rtype) bool {
 					continue
 				}
 			}
-			if i++; i >= len(t.methods) {
+			if i++; i >= len(tmethods) {
 				return true
 			}
 		}
@@ -1658,7 +1661,7 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool {
 	case Interface:
 		t := (*interfaceType)(unsafe.Pointer(T))
 		v := (*interfaceType)(unsafe.Pointer(V))
-		if len(t.methods) == 0 && len(v.methods) == 0 {
+		if t.isEmpty() && v.isEmpty() {
 			return true
 		}
 		// Might have the same methods but still
@@ -2442,7 +2445,7 @@ func StructOf(fields []StructField) Type {
 			switch f.typ.Kind() {
 			case Interface:
 				ift := (*interfaceType)(unsafe.Pointer(ft))
-				for im, m := range ift.methods {
+				for im, m := range ift.methods() {
 					if ift.nameOff(m.name).pkgPath() != "" {
 						// TODO(sbinet).  Issue 15924.
 						panic("reflect: embedded interface with unexported method(s) not implemented")
@@ -3149,3 +3152,11 @@ func addTypeBits(bv *bitVector, offset uintptr, t *rtype) {
 		}
 	}
 }
+
+func isEmptyIface(rt *rtype) bool {
+	if rt.Kind() != Interface {
+		return false
+	}
+	tt := (*interfaceType)(unsafe.Pointer(rt))
+	return len(tt.methods()) == 0
+}