mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

go/types: set correct Var.scopePos for parameters/results 

Previously, its value was unset (NoPos), but the correct
value is a point after the signature (FuncType.End) and
before the body.

Also, fix a bug in Scope.Innermost whereby it would return
the wrong (outer) scope when the query position was in
the FuncType portion of a Func{Decl,Lit}.
The fix is to set the scope's pos/end to those of the
complete Func{Decl,Lit}. This is now documented at
Info.Scopes, along with other missing information.

Also, fix a bug in the go/types (but not types2) scope
test, in which comments were discarded by the parser,
causing the entire test to be a no-op (!).

Also, make failures of TestScopeLookupParent more
informative.

Also, add a release note about the change in behavior.

Fixes #64292
Fixes #64295

Change-Id: Ib681f59d1b0b43de977666db08302d7524d3305f
Reviewed-on: https://go-review.googlesource.com/c/go/+/544035
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Robert Griesemer <gri@google.com>
This commit is contained in:
Alan Donovan 2023-11-20 17:22:37 -05:00 committed by Gopher Robot
parent 858cd8da56
commit a27a525d1b
16 changed files with 169 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
doc/go1.22.html
View File

@ -486,6 +486,18 @@ Do not send CLs removing the interior tags from such phrases.
</dd>
</dl><!-- os/exec -->
<dl id="go/types"><dt><a href="/pkg/go/types/">go/types</a></dt>
<dd>
<p><!-- https://go.dev/issue/64295, CL 544035 -->
The start position (<a href="/pkg/go/types#Scope.Pos">Pos</a>)
of the lexical environment block (<a href="/pkg/go/types#Scope">Scope</a>)
that represents a function body has changed:
it used to start at the opening curly brace of the function body,
but now starts at the function's <code>func</code> token.
</p>
</dd>
</dl>
<dl id="reflect"><dt><a href="/pkg/reflect/">reflect</a></dt>
<dd>
<p><!-- https://go.dev/issue/61827, CL 517777 -->

9
src/cmd/compile/internal/types2/api.go
View File

@ -268,6 +268,15 @@ type Info struct {
// scope, the function scopes are embedded in the file scope of the file
// containing the function declaration.
//
// The Scope of a function contains the declarations of any
// type parameters, parameters, and named results, plus any
// local declarations in the body block.
// It is coextensive with the complete extent of the
// function's syntax ([*ast.FuncDecl] or [*ast.FuncLit]).
// The Scopes mapping does not contain an entry for the
// function body ([*ast.BlockStmt]); the function's scope is
// associated with the [*ast.FuncType].
//
// The following node types may appear in Scopes:
//
// *syntax.File

32
src/cmd/compile/internal/types2/api_test.go
View File

@ -1892,12 +1892,12 @@ const Pi = 3.1415
type T struct{}
var Y, _ = lib.X, X
func F(){
func F[T *U, U any](param1, param2 int) /*param1=undef*/ (res1 /*res1=undef*/, res2 int) /*param1=var:12*/ /*res1=var:12*/ /*U=typename:12*/ {
const pi, e = 3.1415, /*pi=undef*/ 2.71828 /*pi=const:13*/ /*e=const:13*/
type /*t=undef*/ t /*t=typename:14*/ *t
print(Y) /*Y=var:10*/
x, Y := Y, /*x=undef*/ /*Y=var:10*/ Pi /*x=var:16*/ /*Y=var:16*/ ; _ = x; _ = Y
var F = /*F=func:12*/ F /*F=var:17*/ ; _ = F
var F = /*F=func:12*/ F[*int, int] /*F=var:17*/ ; _ = F
var a []int
for i, x := range a /*i=undef*/ /*x=var:16*/ { _ = i; _ = x }
@ -1916,6 +1916,10 @@ func F(){
println(int)
default /*int=var:31*/ :
}
_ = param1
_ = res1
return
}
/*main=undef*/
`
@ -1981,7 +1985,29 @@ func F(){
_, gotObj := inner.LookupParent(id.Value, id.Pos())
if gotObj != wantObj {
t.Errorf("%s: got %v, want %v", id.Pos(), gotObj, wantObj)
// Print the scope tree of mainScope in case of error.
var printScopeTree func(indent string, s *Scope)
printScopeTree = func(indent string, s *Scope) {
t.Logf("%sscope %s %v-%v = %v",
indent,
ScopeComment(s),
s.Pos(),
s.End(),
s.Names())
for i := range s.NumChildren() {
printScopeTree(indent+" ", s.Child(i))
}
}
printScopeTree("", mainScope)
t.Errorf("%s: Scope(%s).LookupParent(%s@%v) got %v, want %v [scopePos=%v]",
id.Pos(),
ScopeComment(inner),
id.Value,
id.Pos(),
gotObj,
wantObj,
ObjectScopePos(wantObj))
continue
}
}

18
src/cmd/compile/internal/types2/decl.go
View File

@ -570,8 +570,11 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list []*syntax.Fiel
// Declare type parameters up-front.
// The scope of type parameters starts at the beginning of the type parameter
// list (so we can have mutually recursive parameterized type bounds).
for i, f := range list {
tparams[i] = check.declareTypeParam(f.Name)
if len(list) > 0 {
scopePos := list[0].Pos()
for i, f := range list {
tparams[i] = check.declareTypeParam(f.Name, scopePos)
}
}
// Set the type parameters before collecting the type constraints because
@ -628,7 +631,7 @@ func (check *Checker) bound(x syntax.Expr) Type {
return check.typ(x)
}
func (check *Checker) declareTypeParam(name *syntax.Name) *TypeParam {
func (check *Checker) declareTypeParam(name *syntax.Name, scopePos syntax.Pos) *TypeParam {
// Use Typ[Invalid] for the type constraint to ensure that a type
// is present even if the actual constraint has not been assigned
// yet.
@ -636,8 +639,8 @@ func (check *Checker) declareTypeParam(name *syntax.Name) *TypeParam {
// constraints to make sure we don't rely on them if they
// are not properly set yet.
tname := NewTypeName(name.Pos(), check.pkg, name.Value, nil)
tpar := check.newTypeParam(tname, Typ[Invalid]) // assigns type to tname as a side-effect
check.declare(check.scope, name, tname, check.scope.pos) // TODO(gri) check scope position
tpar := check.newTypeParam(tname, Typ[Invalid]) // assigns type to tname as a side-effect
check.declare(check.scope, name, tname, scopePos)
return tpar
}
@ -750,6 +753,11 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) {
check.funcType(sig, fdecl.Recv, fdecl.TParamList, fdecl.Type)
obj.color_ = saved
// Set the scope's extent to the complete "func (...) { ... }"
// so that Scope.Innermost works correctly.
sig.scope.pos = fdecl.Pos()
sig.scope.end = syntax.EndPos(fdecl)
if len(fdecl.TParamList) > 0 && fdecl.Body == nil {
check.softErrorf(fdecl, BadDecl, "generic function is missing function body")
}

4
src/cmd/compile/internal/types2/expr.go
View File

@ -1081,6 +1081,10 @@ func (check *Checker) exprInternal(T Type, x *operand, e syntax.Expr, hint Type)
case *syntax.FuncLit:
if sig, ok := check.typ(e.Type).(*Signature); ok {
// Set the Scope's extent to the complete "func (...) {...}"
// so that Scope.Innermost works correctly.
sig.scope.pos = e.Pos()
sig.scope.end = syntax.EndPos(e)
if !check.conf.IgnoreFuncBodies && e.Body != nil {
// Anonymous functions are considered part of the
// init expression/func declaration which contains

28
src/cmd/compile/internal/types2/signature.go
View File

@ -108,9 +108,12 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams []
// - the receiver specification acts as local declaration for its type parameters, which may be blank
_, rname, rparams := check.unpackRecv(recvPar.Type, true)
if len(rparams) > 0 {
// The scope of the type parameter T in "func (r T[T]) f()"
// starts after f, not at "r"; see #52038.
scopePos := ftyp.Pos()
tparams := make([]*TypeParam, len(rparams))
for i, rparam := range rparams {
tparams[i] = check.declareTypeParam(rparam)
tparams[i] = check.declareTypeParam(rparam, scopePos)
}
sig.rparams = bindTParams(tparams)
// Blank identifiers don't get declared, so naive type-checking of the
@ -167,16 +170,21 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams []
check.collectTypeParams(&sig.tparams, tparams)
}
// Value (non-type) parameters' scope starts in the function body. Use a temporary scope for their
// declarations and then squash that scope into the parent scope (and report any redeclarations at
// that time).
// Use a temporary scope for all parameter declarations and then
// squash that scope into the parent scope (and report any
// redeclarations at that time).
//
// TODO(adonovan): now that each declaration has the correct
// scopePos, there should be no need for scope squashing.
// Audit to ensure all lookups honor scopePos and simplify.
scope := NewScope(check.scope, nopos, nopos, "function body (temp. scope)")
var recvList []*Var // TODO(gri) remove the need for making a list here
scopePos := syntax.EndPos(ftyp) // all parameters' scopes start after the signature
var recvList []*Var // TODO(gri) remove the need for making a list here
if recvPar != nil {
recvList, _ = check.collectParams(scope, []*syntax.Field{recvPar}, false) // use rewritten receiver type, if any
recvList, _ = check.collectParams(scope, []*syntax.Field{recvPar}, false, scopePos) // use rewritten receiver type, if any
}
params, variadic := check.collectParams(scope, ftyp.ParamList, true)
results, _ := check.collectParams(scope, ftyp.ResultList, false)
params, variadic := check.collectParams(scope, ftyp.ParamList, true, scopePos)
results, _ := check.collectParams(scope, ftyp.ResultList, false, scopePos)
scope.Squash(func(obj, alt Object) {
var err error_
err.code = DuplicateDecl
@ -259,7 +267,7 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams []
// collectParams declares the parameters of list in scope and returns the corresponding
// variable list.
func (check *Checker) collectParams(scope *Scope, list []*syntax.Field, variadicOk bool) (params []*Var, variadic bool) {
func (check *Checker) collectParams(scope *Scope, list []*syntax.Field, variadicOk bool, scopePos syntax.Pos) (params []*Var, variadic bool) {
if list == nil {
return
}
@ -294,7 +302,7 @@ func (check *Checker) collectParams(scope *Scope, list []*syntax.Field, variadic
// ok to continue
}
par := NewParam(field.Name.Pos(), check.pkg, name, typ)
check.declare(scope, field.Name, par, scope.pos)
check.declare(scope, field.Name, par, scopePos)
params = append(params, par)
named = true
} else {

4
src/cmd/compile/internal/types2/stmt.go
View File

@ -23,10 +23,6 @@ func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body
check.trace(body.Pos(), "-- %s: %s", name, sig)
}
// set function scope extent
sig.scope.pos = body.Pos()
sig.scope.end = syntax.EndPos(body)
// save/restore current environment and set up function environment
// (and use 0 indentation at function start)
defer func(env environment, indent int) {

7
src/cmd/compile/internal/types2/util_test.go
View File

@ -7,6 +7,11 @@
package types2
import "cmd/compile/internal/syntax"
import (
"cmd/compile/internal/syntax"
)
func CmpPos(p, q syntax.Pos) int { return cmpPos(p, q) }
func ScopeComment(s *Scope) string { return s.comment }
func ObjectScopePos(obj Object) syntax.Pos { return obj.scopePos() }

9
src/go/types/api.go
View File

@ -263,6 +263,15 @@ type Info struct {
// scope, the function scopes are embedded in the file scope of the file
// containing the function declaration.
//
// The Scope of a function contains the declarations of any
// type parameters, parameters, and named results, plus any
// local declarations in the body block.
// It is coextensive with the complete extent of the
// function's syntax ([*ast.FuncDecl] or [*ast.FuncLit]).
// The Scopes mapping does not contain an entry for the
// function body ([*ast.BlockStmt]); the function's scope is
// associated with the [*ast.FuncType].
//
// The following node types may appear in Scopes:
//
// *ast.File

35
src/go/types/api_test.go
View File

@ -27,7 +27,7 @@ import (
var nopos token.Pos
func mustParse(fset *token.FileSet, src string) *ast.File {
f, err := parser.ParseFile(fset, pkgName(src), src, 0)
f, err := parser.ParseFile(fset, pkgName(src), src, parser.ParseComments)
if err != nil {
panic(err) // so we don't need to pass *testing.T
}
@ -1896,12 +1896,12 @@ const Pi = 3.1415
type T struct{}
var Y, _ = lib.X, X
func F(){
func F[T *U, U any](param1, param2 int) /*param1=undef*/ (res1 /*res1=undef*/, res2 int) /*param1=var:12*/ /*res1=var:12*/ /*U=typename:12*/ {
const pi, e = 3.1415, /*pi=undef*/ 2.71828 /*pi=const:13*/ /*e=const:13*/
type /*t=undef*/ t /*t=typename:14*/ *t
print(Y) /*Y=var:10*/
x, Y := Y, /*x=undef*/ /*Y=var:10*/ Pi /*x=var:16*/ /*Y=var:16*/ ; _ = x; _ = Y
var F = /*F=func:12*/ F /*F=var:17*/ ; _ = F
var F = /*F=func:12*/ F[*int, int] /*F=var:17*/ ; _ = F
var a []int
for i, x := range a /*i=undef*/ /*x=var:16*/ { _ = i; _ = x }
@ -1920,6 +1920,10 @@ func F(){
println(int)
default /*int=var:31*/ :
}
_ = param1
_ = res1
return
}
/*main=undef*/
`
@ -1981,8 +1985,29 @@ func F(){
_, gotObj := inner.LookupParent(id.Name, id.Pos())
if gotObj != wantObj {
t.Errorf("%s: got %v, want %v",
fset.Position(id.Pos()), gotObj, wantObj)
// Print the scope tree of mainScope in case of error.
var printScopeTree func(indent string, s *Scope)
printScopeTree = func(indent string, s *Scope) {
t.Logf("%sscope %s %v-%v = %v",
indent,
ScopeComment(s),
s.Pos(),
s.End(),
s.Names())
for i := range s.NumChildren() {
printScopeTree(indent+" ", s.Child(i))
}
}
printScopeTree("", mainScope)
t.Errorf("%s: Scope(%s).LookupParent(%s@%v) got %v, want %v [scopePos=%v]",
fset.Position(id.Pos()),
ScopeComment(inner),
id.Name,
id.Pos(),
gotObj,
wantObj,
ObjectScopePos(wantObj))
continue
}
}

14
src/go/types/decl.go
View File

@ -638,8 +638,9 @@ func (check *Checker) collectTypeParams(dst **TypeParamList, list *ast.FieldList
// Declare type parameters up-front, with empty interface as type bound.
// The scope of type parameters starts at the beginning of the type parameter
// list (so we can have mutually recursive parameterized interfaces).
scopePos := list.Pos()
for _, f := range list.List {
tparams = check.declareTypeParams(tparams, f.Names)
tparams = check.declareTypeParams(tparams, f.Names, scopePos)
}
// Set the type parameters before collecting the type constraints because
@ -708,7 +709,7 @@ func (check *Checker) bound(x ast.Expr) Type {
return check.typ(x)
}
func (check *Checker) declareTypeParams(tparams []*TypeParam, names []*ast.Ident) []*TypeParam {
func (check *Checker) declareTypeParams(tparams []*TypeParam, names []*ast.Ident, scopePos token.Pos) []*TypeParam {
// Use Typ[Invalid] for the type constraint to ensure that a type
// is present even if the actual constraint has not been assigned
// yet.
@ -717,8 +718,8 @@ func (check *Checker) declareTypeParams(tparams []*TypeParam, names []*ast.Ident
// are not properly set yet.
for _, name := range names {
tname := NewTypeName(name.Pos(), check.pkg, name.Name, nil)
tpar := check.newTypeParam(tname, Typ[Invalid]) // assigns type to tpar as a side-effect
check.declare(check.scope, name, tname, check.scope.pos) // TODO(gri) check scope position
tpar := check.newTypeParam(tname, Typ[Invalid]) // assigns type to tpar as a side-effect
check.declare(check.scope, name, tname, scopePos)
tparams = append(tparams, tpar)
}
@ -835,6 +836,11 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) {
check.funcType(sig, fdecl.Recv, fdecl.Type)
obj.color_ = saved
// Set the scope's extent to the complete "func (...) { ... }"
// so that Scope.Innermost works correctly.
sig.scope.pos = fdecl.Pos()
sig.scope.end = fdecl.End()
if fdecl.Type.TypeParams.NumFields() > 0 && fdecl.Body == nil {
check.softErrorf(fdecl.Name, BadDecl, "generic function is missing function body")
}

2
src/go/types/eval_test.go
View File

@ -139,7 +139,7 @@ func TestEvalPos(t *testing.T) {
/* c => , struct{c int} */
_ = c
}
_ = func(a, b, c int) /* c => , string */ {
_ = func(a, b, c int /* c => , string */) /* c => , int */ {
/* c => , int */
}
_ = c

4
src/go/types/expr.go
View File

@ -1059,6 +1059,10 @@ func (check *Checker) exprInternal(T Type, x *operand, e ast.Expr, hint Type) ex
case *ast.FuncLit:
if sig, ok := check.typ(e.Type).(*Signature); ok {
// Set the Scope's extent to the complete "func (...) {...}"
// so that Scope.Innermost works correctly.
sig.scope.pos = e.Pos()
sig.scope.end = e.End()
if !check.conf.IgnoreFuncBodies && e.Body != nil {
// Anonymous functions are considered part of the
// init expression/func declaration which contains

27
src/go/types/signature.go
View File

@ -7,6 +7,7 @@ package types
import (
"fmt"
"go/ast"
"go/token"
. "internal/types/errors"
)
@ -115,7 +116,10 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast
// - the receiver specification acts as local declaration for its type parameters, which may be blank
_, rname, rparams := check.unpackRecv(recvPar.List[0].Type, true)
if len(rparams) > 0 {
tparams := check.declareTypeParams(nil, rparams)
// The scope of the type parameter T in "func (r T[T]) f()"
// starts after f, not at "r"; see #52038.
scopePos := ftyp.Params.Pos()
tparams := check.declareTypeParams(nil, rparams, scopePos)
sig.rparams = bindTParams(tparams)
// Blank identifiers don't get declared, so naive type-checking of the
// receiver type expression would fail in Checker.collectParams below,
@ -176,13 +180,18 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast
}
}
// Value (non-type) parameters' scope starts in the function body. Use a temporary scope for their
// declarations and then squash that scope into the parent scope (and report any redeclarations at
// that time).
// Use a temporary scope for all parameter declarations and then
// squash that scope into the parent scope (and report any
// redeclarations at that time).
//
// TODO(adonovan): now that each declaration has the correct
// scopePos, there should be no need for scope squashing.
// Audit to ensure all lookups honor scopePos and simplify.
scope := NewScope(check.scope, nopos, nopos, "function body (temp. scope)")
recvList, _ := check.collectParams(scope, recvPar, false)
params, variadic := check.collectParams(scope, ftyp.Params, true)
results, _ := check.collectParams(scope, ftyp.Results, false)
scopePos := ftyp.End() // all parameters' scopes start after the signature
recvList, _ := check.collectParams(scope, recvPar, false, scopePos)
params, variadic := check.collectParams(scope, ftyp.Params, true, scopePos)
results, _ := check.collectParams(scope, ftyp.Results, false, scopePos)
scope.squash(func(obj, alt Object) {
check.errorf(obj, DuplicateDecl, "%s redeclared in this block", obj.Name())
check.reportAltDecl(alt)
@ -262,7 +271,7 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast
// collectParams declares the parameters of list in scope and returns the corresponding
// variable list.
func (check *Checker) collectParams(scope *Scope, list *ast.FieldList, variadicOk bool) (params []*Var, variadic bool) {
func (check *Checker) collectParams(scope *Scope, list *ast.FieldList, variadicOk bool, scopePos token.Pos) (params []*Var, variadic bool) {
if list == nil {
return
}
@ -290,7 +299,7 @@ func (check *Checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
// ok to continue
}
par := NewParam(name.Pos(), check.pkg, name.Name, typ)
check.declare(scope, name, par, scope.pos)
check.declare(scope, name, par, scopePos)
params = append(params, par)
}
named = true

4
src/go/types/stmt.go
View File

@ -24,10 +24,6 @@ func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body
check.trace(body.Pos(), "-- %s: %s", name, sig)
}
// set function scope extent
sig.scope.pos = body.Pos()
sig.scope.end = body.End()
// save/restore current environment and set up function environment
// (and use 0 indentation at function start)
defer func(env environment, indent int) {

7
src/go/types/util_test.go
View File

@ -9,6 +9,11 @@
package types
import "go/token"
import (
"go/token"
)
func CmpPos(p, q token.Pos) int { return cmpPos(p, q) }
func ScopeComment(s *Scope) string { return s.comment }
func ObjectScopePos(obj Object) token.Pos { return obj.scopePos() }