ignore embedded fields used in reflection (#768)

Fixes #765.

AUTHORS

@@ -14,3 +14,4 @@ Zachary Wasserman <zachwass2000@gmail.com>
 lu4p <lu4p@pm.me>
 pagran <pagran@protonmail.com>
 shellhazard <shellhazard@tutanota.com>
+xuannv <xuan11290@gmail.com>

reflect.go

@@ -406,32 +406,27 @@ func (ri *reflectInspector) recursivelyRecordUsedForReflect(t types.Type) {
 // TODO: consider caching recordedObjectString via a map,
 // if that shows an improvement in our benchmark
 func recordedObjectString(obj types.Object) objectString {
+	// For exported fields, "pkgpath.Field" is not unique,
+	// because two exported top-level types could share "Field".
+	//
+	// Moreover, note that not all fields belong to named struct types;
+	// an API could be exposing:
+	//
+	//   var usedInReflection = struct{Field string}
+	//
+	// For now, a hack: assume that packages don't declare the same field
+	// more than once in the same line. This works in practice, but one
+	// could craft Go code to break this assumption.
+	// Also note that the compiler's object files include filenames and line
+	// numbers, but not column numbers nor byte offsets.
+	// TODO(mvdan): give this another think, and add tests involving anon types.
+	// Note that fields are never top-level.
 	pkg := obj.Pkg()
-	if obj, ok := obj.(*types.Var); ok && obj.IsField() {
-		// For exported fields, "pkgpath.Field" is not unique,
-		// because two exported top-level types could share "Field".
-		//
-		// Moreover, note that not all fields belong to named struct types;
-		// an API could be exposing:
-		//
-		//   var usedInReflection = struct{Field string}
-		//
-		// For now, a hack: assume that packages don't declare the same field
-		// more than once in the same line. This works in practice, but one
-		// could craft Go code to break this assumption.
-		// Also note that the compiler's object files include filenames and line
-		// numbers, but not column numbers nor byte offsets.
-		// TODO(mvdan): give this another think, and add tests involving anon types.
+	if pkg.Scope() != obj.Parent() {
 		pos := fset.Position(obj.Pos())
 		return fmt.Sprintf("%s.%s - %s:%d", pkg.Path(), obj.Name(),
 			filepath.Base(pos.Filename), pos.Line)
 	}
-	// Names which are not at the top level cannot be imported,
-	// so we don't need to record them either.
-	// Note that this doesn't apply to fields, which are never top-level.
-	if pkg.Scope() != obj.Parent() {
-		return ""
-	}
 	// For top-level exported names, "pkgpath.Name" is unique.
 	return pkg.Path() + "." + obj.Name()
 }
@@ -442,20 +437,44 @@ func (ri *reflectInspector) recordUsedForReflect(obj types.Object) {
 	if obj.Pkg().Path() != ri.pkg.Path() {
 		panic("called recordUsedForReflect with a foreign object")
 	}
-	objStr := recordedObjectString(obj)
-	if objStr == "" {
-		// If the object can't be described via a qualified string,
-		// do we need to record it at all?
+
+	if obj, ok := obj.(*types.Var); ok && obj.IsField() {
+		ri.result.ReflectObjects[recordedObjectString(obj)] = struct{}{}
+
+		if !obj.Embedded() {
+			return
+		}
+
+		embeddedType, ok := obj.Type().(*types.Named)
+		if !ok {
+			return
+		}
+
+		embeddedObj := embeddedType.Obj()
+		if embeddedObj.Pkg().Scope() == embeddedObj.Parent() {
+			// not local type
+			return
+		}
+
+		if embeddedObj.Pkg() == nil || embeddedObj.Pkg() != ri.pkg {
+			// not from the specified package
+			return
+		}
+
+		ri.result.ReflectObjects[recordedObjectString(embeddedObj)] = struct{}{}
+
 		return
 	}
-	ri.result.ReflectObjects[objStr] = struct{}{}
+
+	// we don't need to record the local type names
+	if obj.Pkg().Scope() != obj.Parent() {
+		return
+	}
+
+	ri.result.ReflectObjects[recordedObjectString(obj)] = struct{}{}
 }
 
 func usedForReflect(cache pkgCache, obj types.Object) bool {
-	objStr := recordedObjectString(obj)
-	if objStr == "" {
-		return false
-	}
-	_, ok := cache.ReflectObjects[objStr]
+	_, ok := cache.ReflectObjects[recordedObjectString(obj)]
 	return ok
 }

testdata/script/reflect.txtar

@@ -29,6 +29,7 @@ import (
 	"math/big"
 	"os"
 	"reflect"
+	"unsafe"
 	"strings"
 	"text/template"
 
@@ -115,7 +116,7 @@ func main() {
 	printfWithoutPackage("%#v\n", variadic)
 
 	testx509()
-
+	testGoSpew()
 
 	// Very complex reflection used by gorm
 	user := StatUser{}
@@ -175,13 +176,54 @@ type FmtType struct {
 }
 
 // copied from github.com/davecgh/go-spew, which reaches into reflect's internals
-var _ = func() uintptr {
-	field, ok := reflect.TypeOf(reflect.Value{}).FieldByName("flag")
-	if !ok {
-		panic("reflect.Value has no flag field")
+func testGoSpew() {
+	flagValOffset := func() uintptr {
+		field, ok := reflect.TypeOf(reflect.Value{}).FieldByName("flag")
+		if !ok {
+			panic("reflect.Value has no flag field")
+		}
+		return field.Offset
+	}()
+
+	type flag uintptr
+	flagField := func(v *reflect.Value) *flag {
+		return (*flag)(unsafe.Pointer(uintptr(unsafe.Pointer(v)) + flagValOffset))
+	}
+
+	type t0 int
+	var t struct {
+		A t0
+		t0
+		a t0
+	}
+	vA := reflect.ValueOf(t).FieldByName("A")
+	va := reflect.ValueOf(t).FieldByName("a")
+	vt0 := reflect.ValueOf(t).FieldByName("t0")
+	flagvA := *flagField(&vA)
+	flagva := *flagField(&va)
+	flagvt0 := *flagField(&vt0)
+
+	if flagvA&flagva&flagvt0 == 0 {
+		panic("reflect.Value read-only flag has changed semantics")
 	}
-	return field.Offset
-}()
+
+	type T0 int
+	var T struct {
+		A T0
+		T0
+		a T0
+	}
+	vA = reflect.ValueOf(T).FieldByName("A")
+	va = reflect.ValueOf(T).FieldByName("a")
+	vt0 = reflect.ValueOf(T).FieldByName("T0")
+	flagvA = *flagField(&vA)
+	flagva = *flagField(&va)
+	flagvt0 = *flagField(&vt0)
+
+	if flagvA&flagva&flagvt0 == 0 {
+		panic("reflect.Value read-only flag has changed semantics")
+	}
+}
 
 // encoding/x509 uses encoding/asn1, which uses reflect.
 // In one place it depends on field names; that used to be broken by garble.