go/types: use function body scope for type and ordinary parameters

Change-Id: Iab3cf89125d9e13e1dff86710b313770a520a54a

src/go/types/NOTES

@@ -2,8 +2,6 @@ This file works as a sort of notebook/implementation log. It replaces my noteboo
 so we have a better track record. I only switched to this file recently, hence it is incomplete.
 
 TODO
-- Are function type parameters in the same scope as other parameters? TBD.
-  (Right now they are in an enclosing scope.)
 - use Underlying() to return a type parameter's bound? investigate!
 - better error message when declaring a contract local to a function (parser gets out of sync)
 - if type parameters are repeated in recursive instantiation, they must be the same order (not yet checked)
@@ -51,3 +49,8 @@ DESIGN/IMPLEMENTATION
 
 - 12/20/2019: Type parameters may be part of type lists in contracts/interfaces. It just falls out
   naturally. Added test cases.
+
+- 12/23/2019: Decision: Type parameters and ordinary (value) parameters are in the same block, notably
+  the function block. The scope of type parameters starts at the 'type' keyword; the scope of ordinary
+  parameters starts with the (opening '{' of the) function body. Both scopes end with the closing '}'
+  of the function body (i.e., the end of the function block).

src/go/types/scope.go

@@ -108,6 +108,40 @@ func (s *Scope) Insert(obj Object) Object {
 	return nil
 }
 
+// Squash merges s with its parent scope p by adding all
+// objects of s to p, adding all children of s to the
+// children of p, and removing s from p's children.
+// The function f is called for each object obj in s which
+// has an object alt in p. s should be discarded after
+// having been squashed.
+func (s *Scope) Squash(err func(obj, alt Object)) {
+	p := s.parent
+	assert(p != nil)
+	for _, obj := range s.elems {
+		obj.setParent(nil)
+		if alt := p.Insert(obj); alt != nil {
+			err(obj, alt)
+		}
+	}
+
+	j := -1 // index of s in p.children
+	for i, ch := range p.children {
+		if ch == s {
+			j = i
+			break
+		}
+	}
+	assert(j >= 0)
+	k := len(p.children) - 1
+	p.children[j] = p.children[k]
+	p.children = p.children[:k]
+
+	p.children = append(p.children, s.children...)
+
+	s.children = nil
+	s.elems = nil
+}
+
 // Pos and End describe the scope's source code extent [pos, end).
 // The results are guaranteed to be valid only if the type-checked
 // AST has complete position information. The extent is undefined

src/go/types/testdata/tmp.go2

@@ -3,5 +3,3 @@
 // license that can be found in the LICENSE file.
 
 package p
-
-type _(type T int /* ERROR not an interface or contract */ ) struct{}

src/go/types/testdata/typeparams.go2

@@ -7,8 +7,7 @@ package p
 func identity(type T)(x T) T { return x }
 
 func _(type)(x int) int
-func _(type T)(T T) T { return T }
-func _(type T)(T T) T { var x T /* ERROR T.*is not a type */ ; return x }
+func _(type T)(T /* ERROR redeclared */ T)()
 func _(type T, T /* ERROR redeclared */ )()
 
 func reverse(type T)(list []T) []T {

src/go/types/type.go

@@ -524,7 +524,7 @@ func (t *Named) AddMethod(m *Func) {
 
 // A TypeParam represents a type parameter type.
 type TypeParam struct {
-	id    uint64    // unique id (TODO should this be with the object? all objects?)
+	id    uint64    // unique id
 	obj   *TypeName // corresponding type name
 	index int       // parameter index
 	bound Type      // *Named or *Interface; underlying type is always *Interface

src/go/types/typexpr.go

@@ -158,13 +158,15 @@ func (check *Checker) genericType(e ast.Expr) Type {
 
 // funcType type-checks a function or method type.
 func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftparams *ast.FieldList, ftyp *ast.FuncType) {
+	check.openScope(ftyp, "function")
+	check.scope.isFunc = true
+	check.recordScope(ftyp, check.scope)
+	sig.scope = check.scope
+	defer check.closeScope()
+
 	if recvPar != nil && len(recvPar.List) > 0 {
 		_, rname, rparams := check.unpackRecv(recvPar.List[0].Type, true)
 		if len(rparams) > 0 {
-			// declare the method's receiver type parameters
-			check.openScope(recvPar, "receiver type parameters")
-			defer check.closeScope()
-			// collect and declare the type parameters
 			sig.rparams = check.declareTypeParams(nil, rparams, nil)
 			// determine receiver type to get its type parameters
 			// and the respective type parameter bounds
@@ -199,19 +201,20 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftparams
 	}
 
 	if ftparams != nil {
-		// TODO(gri) should this be the same scope as for value parameters?
-		check.openScope(ftparams, "function type parameters")
-		defer check.closeScope()
 		sig.tparams = check.collectTypeParams(ftparams)
 	}
 
-	scope := NewScope(check.scope, token.NoPos, token.NoPos, "function")
-	scope.isFunc = true
-	check.recordScope(ftyp, scope)
-
+	// Value (non-type) parameters' scope starts in the function body. Use a temporary scope for their
+	// declarations and the squash that scope into the parent scope (and report any redeclarations at
+	// at that time).
+	scope := NewScope(check.scope, token.NoPos, token.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)
+	scope.Squash(func(obj, alt Object) {
+		check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name())
+		check.reportAltDecl(alt)
+	})
 
 	if recvPar != nil {
 		// recv parameter list present (may be empty)
@@ -262,7 +265,6 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftparams
 		sig.recv = recv
 	}
 
-	sig.scope = scope
 	sig.params = NewTuple(params...)
 	sig.results = NewTuple(results...)
 	sig.variadic = variadic