From: Robert Griesemer Date: Thu, 26 Sep 2024 23:56:45 +0000 (-0700) Subject: go/types, types2: remove dependency on Scope.Contains in resolver X-Git-Tag: go1.24rc1~813 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5fe3b31cf898de6fbc4f8ac524e16238a9a85e66;p=gostls13.git go/types, types2: remove dependency on Scope.Contains in resolver In extremely rare cases of receiver base types of the form C.foo where C refers to an `import "C"`, we needed Scope.Contains to lookup the file scope containing the "C" import. Replace the position-dependent Scope.Contains with an explicit scope search that doesn't require a position. Also, make the surrounding code match more closely between go/types and types2. Change-Id: Ic007108928dd8b382a06e2bbf09ef8bd6bd0ff36 Reviewed-on: https://go-review.googlesource.com/c/go/+/616256 LUCI-TryBot-Result: Go LUCI Auto-Submit: Robert Griesemer Reviewed-by: Alan Donovan Reviewed-by: Robert Griesemer --- diff --git a/src/cmd/compile/internal/types2/resolver.go b/src/cmd/compile/internal/types2/resolver.go index c16e8289a2..f328359648 100644 --- a/src/cmd/compile/internal/types2/resolver.go +++ b/src/cmd/compile/internal/types2/resolver.go @@ -218,7 +218,8 @@ func (check *Checker) collectObjects() { recv *syntax.Name // receiver type name } var methods []methodInfo // collected methods with valid receivers and non-blank _ names - var fileScopes []*Scope + + fileScopes := make([]*Scope, len(check.files)) // fileScopes[i] corresponds to check.files[i] for fileNo, file := range check.files { check.version = asGoVersion(check.versions[file.Pos().FileBase()]) @@ -227,7 +228,7 @@ func (check *Checker) collectObjects() { check.recordDef(file.PkgName, nil) fileScope := NewScope(pkg.scope, syntax.StartPos(file), syntax.EndPos(file), check.filename(fileNo)) - fileScopes = append(fileScopes, fileScope) + fileScopes[fileNo] = fileScope check.recordScope(file, fileScope) // determine file directory, necessary to resolve imports @@ -492,17 +493,49 @@ func (check *Checker) collectObjects() { // associate methods with receiver base type name where possible. // Ignore methods that have an invalid receiver. They will be // type-checked later, with regular functions. - if methods != nil { - check.methods = make(map[*TypeName][]*Func) - 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, fileScopes) - if base != nil { - m.obj.hasPtrRecv_ = ptr - check.methods[base] = append(check.methods[base], m.obj) + if methods == nil { + return + } + + // lookupScope returns the file scope which contains the given name, + // or nil if the name is not found in any scope. The search does not + // step inside blocks (function bodies). + // This function is only used in conjuction with import "C", and even + // then only rarely. It doesn't have to be particularly fast. + lookupScope := func(name *syntax.Name) *Scope { + for i, file := range check.files { + found := false + syntax.Inspect(file, func(n syntax.Node) bool { + if found { + return false // we're done + } + switch n := n.(type) { + case *syntax.Name: + if n == name { + found = true + return false + } + case *syntax.BlockStmt: + return false // don't descend into function bodies + } + return true + }) + if found { + return fileScopes[i] } } + return nil + } + + check.methods = make(map[*TypeName][]*Func) + 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, lookupScope) + if base != nil { + m.obj.hasPtrRecv_ = ptr + check.methods[base] = append(check.methods[base], m.obj) + } } } @@ -555,7 +588,7 @@ func (check *Checker) unpackRecv(rtyp syntax.Expr, unpackParams bool) (ptr bool, // 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, fileScopes []*Scope) (ptr bool, base *TypeName) { +func (check *Checker) resolveBaseTypeName(seenPtr bool, typ syntax.Expr, lookupScope func(*syntax.Name) *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 @@ -582,18 +615,15 @@ func (check *Checker) resolveBaseTypeName(seenPtr bool, typ syntax.Expr, fileSco name = typ.Value case *syntax.SelectorExpr: // C.struct_foo is a valid type name for packages using cgo. + // See go.dev/issue/59944. + // TODO(gri) why is it possible to associate methods with C types? // // 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) - } - } + obj := lookupScope(ident).Lookup(ident.Value) // the fileScope must always be found // 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 { diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go index af53dc5851..8cc57dc2de 100644 --- a/src/go/types/resolver.go +++ b/src/go/types/resolver.go @@ -233,7 +233,8 @@ func (check *Checker) collectObjects() { recv *ast.Ident // receiver type name } var methods []methodInfo // collected methods with valid receivers and non-blank _ names - var fileScopes []*Scope + + fileScopes := make([]*Scope, len(check.files)) // fileScopes[i] corresponds to check.files[i] for fileNo, file := range check.files { check.version = asGoVersion(check.versions[file]) @@ -249,7 +250,7 @@ func (check *Checker) collectObjects() { pos, end = token.Pos(f.Base()), token.Pos(f.Base()+f.Size()) } fileScope := NewScope(pkg.scope, pos, end, check.filename(fileNo)) - fileScopes = append(fileScopes, fileScope) + fileScopes[fileNo] = fileScope check.recordScope(file, fileScope) // determine file directory, necessary to resolve imports @@ -485,13 +486,44 @@ func (check *Checker) collectObjects() { // Ignore methods that have an invalid receiver. They will be // type-checked later, with regular functions. if methods == nil { - return // nothing to do + return } + + // lookupScope returns the file scope which contains the given name, + // or nil if the name is not found in any scope. The search does not + // step inside blocks (function bodies). + // This function is only used in conjuction with import "C", and even + // then only rarely. It doesn't have to be particularly fast. + lookupScope := func(name *ast.Ident) *Scope { + for i, file := range check.files { + found := false + ast.Inspect(file, func(n ast.Node) bool { + if found { + return false // we're done + } + switch n := n.(type) { + case *ast.Ident: + if n == name { + found = true + return false + } + case *ast.BlockStmt: + return false // don't descend into function bodies + } + return true + }) + if found { + return fileScopes[i] + } + } + return nil + } + check.methods = make(map[*TypeName][]*Func) 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, fileScopes) + ptr, base := check.resolveBaseTypeName(m.ptr, m.recv, lookupScope) if base != nil { m.obj.hasPtrRecv_ = ptr check.methods[base] = append(check.methods[base], m.obj) @@ -549,7 +581,7 @@ func (check *Checker) unpackRecv(rtyp ast.Expr, unpackParams bool) (ptr bool, ba // 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 ast.Expr, fileScopes []*Scope) (ptr bool, base *TypeName) { +func (check *Checker) resolveBaseTypeName(seenPtr bool, typ ast.Expr, lookupScope func(*ast.Ident) *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 @@ -579,18 +611,15 @@ func (check *Checker) resolveBaseTypeName(seenPtr bool, typ ast.Expr, fileScopes name = typ.Name case *ast.SelectorExpr: // C.struct_foo is a valid type name for packages using cgo. + // See go.dev/issue/59944. + // TODO(gri) why is it possible to associate methods with C types? // // Detect this case, and adjust name so that the correct TypeName is // resolved below. if ident, _ := typ.X.(*ast.Ident); ident != nil && ident.Name == "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.Name) - } - } + obj := lookupScope(ident).Lookup(ident.Name) // the fileScope must always be found // 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 {