go/parser: move type params in scope for the function signature

Type parameter resolution is a bit tricky: type parameters are in the
function scope, but unlike ordinary parameters may reference eachother.
When resolving the function scope, we must be careful about the order in
which objects are resolved and declared.

Using ordering allows us to avoid passing around temporary scopes for
field declarations.

Add a bunch of tests for this behavior, and skip "_" in resolution tests
as it just adds noise.

For #45221

Change-Id: Id080cddce3fd76396bf86ba5aba856aedf64a458
Reviewed-on: https://go-review.googlesource.com/c/go/+/304456
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

1 files changed, 76 insertions, 25 deletions

diff --git a/src/go/parser/resolver.go b/src/go/parser/resolver.go
index 1e357e26df..cf92c7e4f5 100644
--- a/src/go/parser/resolver.go
+++ b/src/go/parser/resolver.go
@@ -19,12 +19,12 @@ const debugResolve = false
 // If declErr is non-nil, it is used to report declaration errors during
 // resolution. tok is used to format position in error messages.
 func resolveFile(file *ast.File, handle *token.File, declErr func(token.Pos, string)) {
-	topScope := ast.NewScope(nil)
+	pkgScope := ast.NewScope(nil)
 	r := &resolver{
 		handle:   handle,
 		declErr:  declErr,
-		topScope: topScope,
-		pkgScope: topScope,
+		topScope: pkgScope,
+		pkgScope: pkgScope,
 	}
 
 	for _, decl := range file.Decls {
@@ -245,9 +245,10 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor {
 		r.resolve(n, true)
 
 	case *ast.FuncLit:
-		functionScope := ast.NewScope(r.topScope)
-		r.walkFuncType(functionScope, n.Type)
-		r.walkBody(functionScope, n.Body)
+		r.openScope(n.Pos())
+		defer r.closeScope()
+		r.walkFuncType(n.Type)
+		r.walkBody(n.Body)
 
 	case *ast.SelectorExpr:
 		ast.Walk(r, n.X)
@@ -255,12 +256,14 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor {
 		// resolution.
 
 	case *ast.StructType:
-		scope := ast.NewScope(nil)
-		r.walkFieldList(scope, n.Fields, ast.Var)
+		r.openScope(n.Pos())
+		defer r.closeScope()
+		r.walkFieldList(n.Fields, ast.Var)
 
 	case *ast.FuncType:
-		scope := ast.NewScope(r.topScope)
-		r.walkFuncType(scope, n)
+		r.openScope(n.Pos())
+		defer r.closeScope()
+		r.walkFuncType(n)
 
 	case *ast.CompositeLit:
 		if n.Type != nil {
@@ -283,8 +286,9 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor {
 		}
 
 	case *ast.InterfaceType:
-		scope := ast.NewScope(nil)
-		r.walkFieldList(scope, n.Methods, ast.Fun)
+		r.openScope(n.Pos())
+		defer r.closeScope()
+		r.walkFieldList(n.Methods, ast.Fun)
 
 	// Statements
 	case *ast.LabeledStmt:
@@ -454,17 +458,36 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor {
 				if tparams := typeparams.Get(spec); tparams != nil {
 					r.openScope(spec.Pos())
 					defer r.closeScope()
-					r.walkFieldList(r.topScope, tparams, ast.Typ)
+					r.walkTParams(tparams)
 				}
 				ast.Walk(r, spec.Type)
 			}
 		}
 
 	case *ast.FuncDecl:
-		scope := ast.NewScope(r.topScope)
-		r.walkFieldList(scope, n.Recv, ast.Var)
-		r.walkFuncType(scope, n.Type)
-		r.walkBody(scope, n.Body)
+		// Open the function scope.
+		r.openScope(n.Pos())
+		defer r.closeScope()
+
+		// Resolve the receiver first, without declaring.
+		r.resolveList(n.Recv)
+
+		// Type parameters are walked normally: they can reference each other, and
+		// can be referenced by normal parameters.
+		if tparams := typeparams.Get(n.Type); tparams != nil {
+			r.walkTParams(tparams)
+			// TODO(rFindley): need to address receiver type parameters.
+		}
+
+		// Resolve and declare parameters in a specific order to get duplicate
+		// declaration errors in the correct location.
+		r.resolveList(n.Type.Params)
+		r.resolveList(n.Type.Results)
+		r.declareList(n.Recv, ast.Var)
+		r.declareList(n.Type.Params, ast.Var)
+		r.declareList(n.Type.Results, ast.Var)
+
+		r.walkBody(n.Body)
 		if n.Recv == nil && n.Name.Name != "init" {
 			r.declare(n, nil, r.pkgScope, ast.Fun, n.Name)
 		}
@@ -476,12 +499,15 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor {
 	return nil
 }
 
-func (r *resolver) walkFuncType(scope *ast.Scope, typ *ast.FuncType) {
-	r.walkFieldList(scope, typ.Params, ast.Var)
-	r.walkFieldList(scope, typ.Results, ast.Var)
+func (r *resolver) walkFuncType(typ *ast.FuncType) {
+	// typ.TParams must be walked separately for FuncDecls.
+	r.resolveList(typ.Params)
+	r.resolveList(typ.Results)
+	r.declareList(typ.Params, ast.Var)
+	r.declareList(typ.Results, ast.Var)
 }
 
-func (r *resolver) walkFieldList(scope *ast.Scope, list *ast.FieldList, kind ast.ObjKind) {
+func (r *resolver) resolveList(list *ast.FieldList) {
 	if list == nil {
 		return
 	}
@@ -489,16 +515,41 @@ func (r *resolver) walkFieldList(scope *ast.Scope, list *ast.FieldList, kind ast
 		if f.Type != nil {
 			ast.Walk(r, f.Type)
 		}
-		r.declare(f, nil, scope, kind, f.Names...)
 	}
 }
 
-func (r *resolver) walkBody(scope *ast.Scope, body *ast.BlockStmt) {
+func (r *resolver) declareList(list *ast.FieldList, kind ast.ObjKind) {
+	if list == nil {
+		return
+	}
+	for _, f := range list.List {
+		r.declare(f, nil, r.topScope, kind, f.Names...)
+	}
+}
+
+func (r *resolver) walkFieldList(list *ast.FieldList, kind ast.ObjKind) {
+	if list == nil {
+		return
+	}
+	r.resolveList(list)
+	r.declareList(list, kind)
+}
+
+// walkTParams is like walkFieldList, but declares type parameters eagerly so
+// that they may be resolved in the constraint expressions held in the field
+// Type.
+func (r *resolver) walkTParams(list *ast.FieldList) {
+	if list == nil {
+		return
+	}
+	r.declareList(list, ast.Typ)
+	r.resolveList(list)
+}
+
+func (r *resolver) walkBody(body *ast.BlockStmt) {
 	if body == nil {
 		return
 	}
-	r.topScope = scope // open function scope
-	defer r.closeScope()
 	r.openLabelScope()
 	defer r.closeLabelScope()
 	r.walkStmts(body.List)