]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: remove dependency on Scope.Contains in resolver
authorRobert Griesemer <gri@golang.org>
Thu, 26 Sep 2024 23:56:45 +0000 (16:56 -0700)
committerGopher Robot <gobot@golang.org>
Fri, 27 Sep 2024 16:45:09 +0000 (16:45 +0000)
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
src/cmd/compile/internal/types2/resolver.go
src/go/types/resolver.go

index c16e8289a265b8aaea61d9ce65a2ea3db6d8d530..f32835964826483c776bff3eb2389a59eb99b4fb 100644 (file)
@@ -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 {
index af53dc5851716b867c2efd3cf7ab46108c76a807..8cc57dc2dee24d9ab904a1836b7b0c4ac3da0f40 100644 (file)
@@ -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 {