go/types: resolve cgo base type names

When associating methods with their receiver base, we need to implement
the same indirection through Cgo types as is done for selector
expressions. This fixes a bug where methods declared on aliases of Cgo
types were not associated with their receiver.

While porting to types2, align the types2 testFiles helper with the
go/types implementation. In order to avoid call-site bloat, switch to an
options pattern for configuring the Config used to type-check.

Fixes #59944

Change-Id: Id14101f01c122b6c856ae5453bd00ec07e83f414
Reviewed-on: https://go-review.googlesource.com/c/go/+/493877
Reviewed-by: Robert Griesemer <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: findleyr

src/cmd/compile/internal/types2/check_test.go

@@ -37,6 +37,7 @@ import (
 	"internal/testenv"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"strconv"
 	"strings"
@@ -50,12 +51,14 @@ var (
 	verifyErrors = flag.Bool("verify", false, "verify errors (rather than list them) in TestManual")
 )
 
-func parseFiles(t *testing.T, filenames []string, mode syntax.Mode) ([]*syntax.File, []error) {
+func parseFiles(t *testing.T, filenames []string, srcs [][]byte, mode syntax.Mode) ([]*syntax.File, []error) {
 	var files []*syntax.File
 	var errlist []error
 	errh := func(err error) { errlist = append(errlist, err) }
-	for _, filename := range filenames {
-		file, err := syntax.ParseFile(filename, errh, nil, mode)
+	for i, filename := range filenames {
+		base := syntax.NewFileBase(filename)
+		r := bytes.NewReader(srcs[i])
+		file, err := syntax.Parse(base, r, errh, nil, mode)
 		if file == nil {
 			t.Fatalf("%s: %s", filename, err)
 		}
@@ -83,30 +86,10 @@ func absDiff(x, y uint) uint {
 	return x - y
 }
 
-// Note: parseFlags is identical to the version in go/types which is
-//       why it has a src argument even though here it is always nil.
-
-// parseFlags parses flags from the first line of the given source
-// (from src if present, or by reading from the file) if the line
-// starts with "//" (line comment) followed by "-" (possibly with
-// spaces between). Otherwise the line is ignored.
-func parseFlags(filename string, src []byte, flags *flag.FlagSet) error {
-	// If there is no src, read from the file.
-	const maxLen = 256
-	if len(src) == 0 {
-		f, err := os.Open(filename)
-		if err != nil {
-			return err
-		}
-
-		var buf [maxLen]byte
-		n, err := f.Read(buf[:])
-		if err != nil {
-			return err
-		}
-		src = buf[:n]
-	}
-
+// parseFlags parses flags from the first line of the given source if the line
+// starts with "//" (line comment) followed by "-" (possibly with spaces
+// between). Otherwise the line is ignored.
+func parseFlags(src []byte, flags *flag.FlagSet) error {
 	// we must have a line comment that starts with a "-"
 	const prefix = "//"
 	if !bytes.HasPrefix(src, []byte(prefix)) {
@@ -117,14 +100,24 @@ func parseFlags(filename string, src []byte, flags *flag.FlagSet) error {
 		return nil // comment doesn't start with a "-"
 	}
 	end := bytes.Index(src, []byte("\n"))
+	const maxLen = 256
 	if end < 0 || end > maxLen {
 		return fmt.Errorf("flags comment line too long")
 	}
 
 	return flags.Parse(strings.Fields(string(src[:end])))
 }
 
-func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
+// testFiles type-checks the package consisting of the given files, and
+// compares the resulting errors with the ERROR annotations in the source.
+//
+// The srcs slice contains the file content for the files named in the
+// filenames slice. The colDelta parameter specifies the tolerance for position
+// mismatch when comparing errors. The manual parameter specifies whether this
+// is a 'manual' test.
+//
+// If provided, opts may be used to mutate the Config before type-checking.
+func testFiles(t *testing.T, filenames []string, srcs [][]byte, colDelta uint, manual bool, opts ...func(*Config)) {
 	if len(filenames) == 0 {
 		t.Fatal("no source files")
 	}
@@ -133,11 +126,11 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 	flags := flag.NewFlagSet("", flag.PanicOnError)
 	flags.StringVar(&conf.GoVersion, "lang", "", "")
 	flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
-	if err := parseFlags(filenames[0], nil, flags); err != nil {
+	if err := parseFlags(srcs[0], flags); err != nil {
 		t.Fatal(err)
 	}
 
-	files, errlist := parseFiles(t, filenames, 0)
+	files, errlist := parseFiles(t, filenames, srcs, 0)
 
 	pkgName := "<no package>"
 	if len(files) > 0 {
@@ -165,6 +158,11 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 		}
 		errlist = append(errlist, err)
 	}
+
+	for _, opt := range opts {
+		opt(&conf)
+	}
+
 	conf.Check(pkgName, files, nil)
 
 	if listErrors {
@@ -173,16 +171,10 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 
 	// collect expected errors
 	errmap := make(map[string]map[uint][]syntax.Error)
-	for _, filename := range filenames {
-		f, err := os.Open(filename)
-		if err != nil {
-			t.Error(err)
-			continue
-		}
-		if m := syntax.CommentMap(f, regexp.MustCompile("^ ERRORx? ")); len(m) > 0 {
+	for i, filename := range filenames {
+		if m := syntax.CommentMap(bytes.NewReader(srcs[i]), regexp.MustCompile("^ ERRORx? ")); len(m) > 0 {
 			errmap[filename] = m
 		}
-		f.Close()
 	}
 
 	// match against found errors
@@ -280,6 +272,13 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 	}
 }
 
+// boolFieldAddr(conf, name) returns the address of the boolean field conf.<name>.
+// For accessing unexported fields.
+func boolFieldAddr(conf *Config, name string) *bool {
+	v := reflect.Indirect(reflect.ValueOf(conf))
+	return (*bool)(v.FieldByName(name).Addr().UnsafePointer())
+}
+
 // TestManual is for manual testing of a package - either provided
 // as a list of filenames belonging to the package, or a directory
 // name containing the package files - after the test arguments
@@ -314,7 +313,7 @@ func TestManual(t *testing.T) {
 		}
 		testDir(t, filenames[0], 0, true)
 	} else {
-		testFiles(t, filenames, 0, true)
+		testPkg(t, filenames, 0, true)
 	}
 }
 
@@ -351,7 +350,7 @@ func testDirFiles(t *testing.T, dir string, colDelta uint, manual bool) {
 			testDir(t, path, colDelta, manual)
 		} else {
 			t.Run(filepath.Base(path), func(t *testing.T) {
-				testFiles(t, []string{path}, colDelta, manual)
+				testPkg(t, []string{path}, colDelta, manual)
 			})
 		}
 	}
@@ -370,6 +369,18 @@ func testDir(t *testing.T, dir string, colDelta uint, manual bool) {
 	}
 
 	t.Run(filepath.Base(dir), func(t *testing.T) {
-		testFiles(t, filenames, colDelta, manual)
+		testPkg(t, filenames, colDelta, manual)
 	})
 }
+
+func testPkg(t *testing.T, filenames []string, colDelta uint, manual bool) {
+	srcs := make([][]byte, len(filenames))
+	for i, filename := range filenames {
+		src, err := os.ReadFile(filename)
+		if err != nil {
+			t.Fatalf("could not read %s: %v", filename, err)
+		}
+		srcs[i] = src
+	}
+	testFiles(t, filenames, srcs, colDelta, manual)
+}

src/cmd/compile/internal/types2/issues_test.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"internal/testenv"
 	"regexp"
+	"runtime"
 	"sort"
 	"strings"
 	"testing"
@@ -804,3 +805,71 @@ func (S) M5(struct {S;t}) {}
 		test(t.main, t.b, t.want)
 	}
 }
+
+func TestIssue59944(t *testing.T) {
+	if runtime.GOARCH == "wasm" {
+		// While we don't use the cgo tool directly in this test, we must have the
+		// syscall package.
+		t.Skip("cgo generated code does not compile on wasm")
+	}
+	// The typechecker should resolve methods declared on aliases of cgo types.
+	const src = `
+package p
+
+/*
+struct layout {
+	int field;
+};
+*/
+import "C"
+
+type Layout = C.struct_layout
+
+func (l *Layout) Binding() {}
+
+func _() {
+	_ = (*Layout).Binding
+}
+`
+
+	// code generated by cmd/cgo for the above source.
+	const cgoTypes = `
+// Code generated by cmd/cgo; DO NOT EDIT.
+
+package p
+
+import "unsafe"
+
+import "syscall"
+
+import _cgopackage "runtime/cgo"
+
+type _ _cgopackage.Incomplete
+var _ syscall.Errno
+func _Cgo_ptr(ptr unsafe.Pointer) unsafe.Pointer { return ptr }
+
+//go:linkname _Cgo_always_false runtime.cgoAlwaysFalse
+var _Cgo_always_false bool
+//go:linkname _Cgo_use runtime.cgoUse
+func _Cgo_use(interface{})
+type _Ctype_int int32
+
+type _Ctype_struct_layout struct {
+	field _Ctype_int
+}
+
+type _Ctype_void [0]byte
+
+//go:linkname _cgo_runtime_cgocall runtime.cgocall
+func _cgo_runtime_cgocall(unsafe.Pointer, uintptr) int32
+
+//go:linkname _cgoCheckPointer runtime.cgoCheckPointer
+func _cgoCheckPointer(interface{}, interface{})
+
+//go:linkname _cgoCheckResult runtime.cgoCheckResult
+func _cgoCheckResult(interface{})
+`
+	testFiles(t, []string{"p.go", "_cgo_gotypes.go"}, [][]byte{[]byte(src), []byte(cgoTypes)}, 0, false, func(cfg *Config) {
+		*boolFieldAddr(cfg, "go115UsesCgo") = true
+	})
+}

src/cmd/compile/internal/types2/resolver.go

@@ -497,7 +497,7 @@ func (check *Checker) collectObjects() {
 		for i := range methods {
 			m := &methods[i]
 			// Determine the receiver base type and associate m with it.
-			ptr, base := check.resolveBaseTypeName(m.ptr, m.recv)
+			ptr, base := check.resolveBaseTypeName(m.ptr, m.recv, fileScopes)
 			if base != nil {
 				m.obj.hasPtrRecv_ = ptr
 				check.methods[base] = append(check.methods[base], m.obj)
@@ -571,7 +571,7 @@ L: // unpack receiver type
 // there was a pointer indirection to get to it. The base type name must be declared
 // in package scope, and there can be at most one pointer indirection. If no such type
 // name exists, the returned base is nil.
-func (check *Checker) resolveBaseTypeName(seenPtr bool, typ syntax.Expr) (ptr bool, base *TypeName) {
+func (check *Checker) resolveBaseTypeName(seenPtr bool, typ syntax.Expr, fileScopes []*Scope) (ptr bool, base *TypeName) {
 	// Algorithm: Starting from a type expression, which may be a name,
 	// we follow that type through alias declarations until we reach a
 	// non-alias type name. If we encounter anything but pointer types or
@@ -580,8 +580,6 @@ func (check *Checker) resolveBaseTypeName(seenPtr bool, typ syntax.Expr) (ptr bo
 	ptr = seenPtr
 	var seen map[*TypeName]bool
 	for {
-		typ = unparen(typ)
-
 		// check if we have a pointer type
 		// if pexpr, _ := typ.(*ast.StarExpr); pexpr != nil {
 		if pexpr, _ := typ.(*syntax.Operation); pexpr != nil && pexpr.Op == syntax.Mul && pexpr.Y == nil {
@@ -593,15 +591,43 @@ func (check *Checker) resolveBaseTypeName(seenPtr bool, typ syntax.Expr) (ptr bo
 			typ = unparen(pexpr.X) // continue with pointer base type
 		}
 
-		// typ must be a name
-		name, _ := typ.(*syntax.Name)
-		if name == nil {
+		// typ must be a name, or a C.name cgo selector.
+		var name string
+		switch typ := typ.(type) {
+		case *syntax.Name:
+			name = typ.Value
+		case *syntax.SelectorExpr:
+			// C.struct_foo is a valid type name for packages using cgo.
+			//
+			// Detect this case, and adjust name so that the correct TypeName is
+			// resolved below.
+			if ident, _ := typ.X.(*syntax.Name); ident != nil && ident.Value == "C" {
+				// Check whether "C" actually resolves to an import of "C", by looking
+				// in the appropriate file scope.
+				var obj Object
+				for _, scope := range fileScopes {
+					if scope.Contains(ident.Pos()) {
+						obj = scope.Lookup(ident.Value)
+					}
+				}
+				// If Config.go115UsesCgo is set, the typechecker will resolve Cgo
+				// selectors to their cgo name. We must do the same here.
+				if pname, _ := obj.(*PkgName); pname != nil {
+					if pname.imported.cgo { // only set if Config.go115UsesCgo is set
+						name = "_Ctype_" + typ.Sel.Value
+					}
+				}
+			}
+			if name == "" {
+				return false, nil
+			}
+		default:
 			return false, nil
 		}
 
 		// name must denote an object found in the current package scope
 		// (note that dot-imported objects are not in the package scope!)
-		obj := check.pkg.scope.Lookup(name.Value)
+		obj := check.pkg.scope.Lookup(name)
 		if obj == nil {
 			return false, nil
 		}