From: Robert Findley Date: Tue, 1 Feb 2022 22:13:53 +0000 (-0500) Subject: go/parser: fix incorrect resolution of receiver type parameters X-Git-Tag: go1.18rc1~82 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=911c78fe54fe9fc0655c013d2aa303e147d63529;p=gostls13.git go/parser: fix incorrect resolution of receiver type parameters Declare receiver type parameters in the function scope, but don't resolve them (for now), as ast.Object.Decl is not documented to hold *ast.Idents. This avoids incorrect resolution of identifiers to names outside the function scope. Also make tracing and error reporting more consistent. For golang/go#50956 Change-Id: I8cd61dd25f4c0f6b974221599b00e23d8da206a1 Reviewed-on: https://go-review.googlesource.com/c/go/+/382247 Trust: Robert Findley Run-TryBot: Robert Findley Reviewed-by: Robert Griesemer TryBot-Result: Gopher Robot --- diff --git a/src/go/parser/resolver.go b/src/go/parser/resolver.go index 910ca0689c..d66a194c12 100644 --- a/src/go/parser/resolver.go +++ b/src/go/parser/resolver.go @@ -8,6 +8,7 @@ import ( "fmt" "go/ast" "go/token" + "strings" ) const debugResolve = false @@ -24,6 +25,7 @@ func resolveFile(file *ast.File, handle *token.File, declErr func(token.Pos, str declErr: declErr, topScope: pkgScope, pkgScope: pkgScope, + depth: 1, } for _, decl := range file.Decls { @@ -45,7 +47,7 @@ func resolveFile(file *ast.File, handle *token.File, declErr func(token.Pos, str i++ } else if debugResolve { pos := ident.Obj.Decl.(interface{ Pos() token.Pos }).Pos() - r.dump("resolved %s@%v to package object %v", ident.Name, ident.Pos(), pos) + r.trace("resolved %s@%v to package object %v", ident.Name, ident.Pos(), pos) } } file.Scope = r.pkgScope @@ -60,6 +62,7 @@ type resolver struct { pkgScope *ast.Scope // pkgScope.Outer == nil topScope *ast.Scope // top-most scope; may be pkgScope unresolved []*ast.Ident // unresolved identifiers + depth int // scope depth // Label scopes // (maintained by open/close LabelScope) @@ -67,8 +70,8 @@ type resolver struct { targetStack [][]*ast.Ident // stack of unresolved labels } -func (r *resolver) dump(format string, args ...any) { - fmt.Println(">>> " + r.sprintf(format, args...)) +func (r *resolver) trace(format string, args ...any) { + fmt.Println(strings.Repeat(". ", r.depth) + r.sprintf(format, args...)) } func (r *resolver) sprintf(format string, args ...any) string { @@ -83,14 +86,16 @@ func (r *resolver) sprintf(format string, args ...any) string { func (r *resolver) openScope(pos token.Pos) { if debugResolve { - r.dump("opening scope @%v", pos) + r.trace("opening scope @%v", pos) + r.depth++ } r.topScope = ast.NewScope(r.topScope) } func (r *resolver) closeScope() { if debugResolve { - r.dump("closing scope") + r.depth-- + r.trace("closing scope") } r.topScope = r.topScope.Outer } @@ -117,21 +122,27 @@ func (r *resolver) closeLabelScope() { func (r *resolver) declare(decl, data any, scope *ast.Scope, kind ast.ObjKind, idents ...*ast.Ident) { for _, ident := range idents { - assert(ident.Obj == nil, "identifier already declared or resolved") + if ident.Obj != nil { + panic(fmt.Sprintf("%v: identifier %s already declared or resolved", ident.Pos(), ident.Name)) + } obj := ast.NewObj(kind, ident.Name) // remember the corresponding declaration for redeclaration // errors and global variable resolution/typechecking phase obj.Decl = decl obj.Data = data - ident.Obj = obj + // Identifiers (for receiver type parameters) are written to the scope, but + // never set as the resolved object. See issue #50956. + if _, ok := decl.(*ast.Ident); !ok { + ident.Obj = obj + } if ident.Name != "_" { if debugResolve { - r.dump("declaring %s@%v", ident.Name, ident.Pos()) + r.trace("declaring %s@%v", ident.Name, ident.Pos()) } if alt := scope.Insert(obj); alt != nil && r.declErr != nil { prevDecl := "" if pos := alt.Pos(); pos.IsValid() { - prevDecl = fmt.Sprintf("\n\tprevious declaration at %s", r.handle.Position(pos)) + prevDecl = r.sprintf("\n\tprevious declaration at %v", pos) } r.declErr(ident.Pos(), fmt.Sprintf("%s redeclared in this block%s", ident.Name, prevDecl)) } @@ -153,7 +164,7 @@ func (r *resolver) shortVarDecl(decl *ast.AssignStmt) { ident.Obj = obj if ident.Name != "_" { if debugResolve { - r.dump("declaring %s@%v", ident.Name, ident.Pos()) + r.trace("declaring %s@%v", ident.Name, ident.Pos()) } if alt := r.topScope.Insert(obj); alt != nil { ident.Obj = alt // redeclaration @@ -180,7 +191,7 @@ var unresolved = new(ast.Object) // func (r *resolver) resolve(ident *ast.Ident, collectUnresolved bool) { if ident.Obj != nil { - panic(fmt.Sprintf("%s: identifier %s already declared or resolved", r.handle.Position(ident.Pos()), ident.Name)) + panic(r.sprintf("%v: identifier %s already declared or resolved", ident.Pos(), ident.Name)) } // '_' should never refer to existing declarations, because it has special // handling in the spec. @@ -189,8 +200,15 @@ func (r *resolver) resolve(ident *ast.Ident, collectUnresolved bool) { } for s := r.topScope; s != nil; s = s.Outer { if obj := s.Lookup(ident.Name); obj != nil { + if debugResolve { + r.trace("resolved %v:%s to %v", ident.Pos(), ident.Name, obj) + } assert(obj.Name != "", "obj with no name") - ident.Obj = obj + // Identifiers (for receiver type parameters) are written to the scope, + // but never set as the resolved object. See issue #50956. + if _, ok := obj.Decl.(*ast.Ident); !ok { + ident.Obj = obj + } return } } @@ -227,7 +245,7 @@ func (r *resolver) walkStmts(list []ast.Stmt) { func (r *resolver) Visit(node ast.Node) ast.Visitor { if debugResolve && node != nil { - r.dump("node %T@%v", node, node.Pos()) + r.trace("node %T@%v", node, node.Pos()) } switch n := node.(type) { @@ -461,8 +479,7 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor { r.openScope(n.Pos()) defer r.closeScope() - // Resolve the receiver first, without declaring. - r.resolveList(n.Recv) + r.walkRecv(n.Recv) // Type parameters are walked normally: they can reference each other, and // can be referenced by normal parameters. @@ -519,6 +536,52 @@ func (r *resolver) declareList(list *ast.FieldList, kind ast.ObjKind) { } } +func (r *resolver) walkRecv(recv *ast.FieldList) { + // If our receiver has receiver type parameters, we must declare them before + // trying to resolve the rest of the receiver, and avoid re-resolving the + // type parameter identifiers. + if recv == nil || len(recv.List) == 0 { + return // nothing to do + } + typ := recv.List[0].Type + if ptr, ok := typ.(*ast.StarExpr); ok { + typ = ptr.X + } + + var declareExprs []ast.Expr // exprs to declare + var resolveExprs []ast.Expr // exprs to resolve + switch typ := typ.(type) { + case *ast.IndexExpr: + declareExprs = []ast.Expr{typ.Index} + resolveExprs = append(resolveExprs, typ.X) + case *ast.IndexListExpr: + declareExprs = typ.Indices + resolveExprs = append(resolveExprs, typ.X) + default: + resolveExprs = append(resolveExprs, typ) + } + for _, expr := range declareExprs { + if id, _ := expr.(*ast.Ident); id != nil { + r.declare(expr, nil, r.topScope, ast.Typ, id) + } else { + // The receiver type parameter expression is invalid, but try to resolve + // it anyway for consistency. + resolveExprs = append(resolveExprs, expr) + } + } + for _, expr := range resolveExprs { + if expr != nil { + ast.Walk(r, expr) + } + } + // The receiver is invalid, but try to resolve it anyway for consistency. + for _, f := range recv.List[1:] { + if f.Type != nil { + ast.Walk(r, f.Type) + } + } +} + func (r *resolver) walkFieldList(list *ast.FieldList, kind ast.ObjKind) { if list == nil { return diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go index 6ea430636e..cf4fa0a902 100644 --- a/src/go/parser/short_test.go +++ b/src/go/parser/short_test.go @@ -237,6 +237,8 @@ var invalidNoTParamErrs = []string{ `package p; func (T) _[ /* ERROR "expected '\(', found '\['" */ A, B any](a A) B`, `package p; func (T) _[ /* ERROR "expected '\(', found '\['" */ A, B C](a A) B`, `package p; func (T) _[ /* ERROR "expected '\(', found '\['" */ A, B C[A, B]](a A) B`, + + `package p; func(*T[ /* ERROR "missing ',' in parameter list" */ e, e]) _()`, } // invalidTParamErrs holds invalid source code examples annotated with the @@ -255,6 +257,8 @@ var invalidTParamErrs = []string{ `package p; func (T) _[ /* ERROR "must have no type parameters" */ A, B any](a A) B`, `package p; func (T) _[ /* ERROR "must have no type parameters" */ A, B C](a A) B`, `package p; func (T) _[ /* ERROR "must have no type parameters" */ A, B C[A, B]](a A) B`, + + `package p; func(*T[e, e /* ERROR "e redeclared" */ ]) _()`, } func TestInvalid(t *testing.T) { diff --git a/src/go/parser/testdata/resolution/typeparams.go2 b/src/go/parser/testdata/resolution/typeparams.go2 index 8c243afda7..7395ca2a34 100644 --- a/src/go/parser/testdata/resolution/typeparams.go2 +++ b/src/go/parser/testdata/resolution/typeparams.go2 @@ -25,10 +25,15 @@ func Add /* =@AddDecl */[T /* =@T */ Addable /* @Addable */](l /* =@l */, r /* = type Receiver /* =@Receiver */[P /* =@P */ any] struct {} +type RP /* =@RP1 */ struct{} + // TODO(rFindley): make a decision on how/whether to resolve identifiers that // refer to receiver type parameters, as is the case for the 'P' result // parameter below. -func (r /* =@recv */ Receiver /* @Receiver */ [P]) m() P {} +// +// For now, we ensure that types are not incorrectly resolved when receiver +// type parameters are in scope. +func (r /* =@recv */ Receiver /* @Receiver */ [RP]) m(RP) RP {} func f /* =@f */[T1 /* =@T1 */ interface{~[]T2 /* @T2 */}, T2 /* =@T2 */ any]( x /* =@x */ T1 /* @T1 */, T1 /* =@T1_duplicate */ y, // Note that this is a bug: @@ -41,3 +46,6 @@ func f /* =@f */[T1 /* =@T1 */ interface{~[]T2 /* @T2 */}, T2 /* =@T2 */ any]( T1 /* @T1 */ := 0 var t1var /* =@t1var */ T1 /* @T1 */ } + +// From issue #39634 +func(*ph1[e, e])h(d)