go/types, types2: fix scoping for iteration variables declared by range clause

Also correct scope position for such variables.
Adjusted some comments.

Fixes #51437.

Change-Id: Ic49a1459469c8b2c7bc24fe546795f7d56c67cb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/389594
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>

src/cmd/compile/internal/types2/api_test.go

@@ -1699,7 +1699,7 @@ func F(){
 	var F = /*F=func:12*/ F /*F=var:17*/ ; _ = F
 
 	var a []int
-	for i, x := range /*i=undef*/ /*x=var:16*/ a /*i=var:20*/ /*x=var:20*/ { _ = i; _ = x }
+	for i, x := range a /*i=undef*/ /*x=var:16*/ { _ = i; _ = x }
 
 	var i interface{}
 	switch y := i.(type) { /*y=undef*/

src/cmd/compile/internal/types2/stmt.go

@@ -626,14 +626,15 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
 
 	case *syntax.ForStmt:
 		inner |= breakOk | continueOk
-		check.openScope(s, "for")
-		defer check.closeScope()
 
 		if rclause, _ := s.Init.(*syntax.RangeClause); rclause != nil {
 			check.rangeStmt(inner, s, rclause)
 			break
 		}
 
+		check.openScope(s, "for")
+		defer check.closeScope()
+
 		check.simpleStmt(s.Init)
 		if s.Cond != nil {
 			var x operand
@@ -809,8 +810,6 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu
 }
 
 func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *syntax.RangeClause) {
-	// scope already opened
-
 	// determine lhs, if any
 	sKey := rclause.Lhs // possibly nil
 	var sValue, sExtra syntax.Expr
@@ -866,6 +865,11 @@ func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *s
 		}
 	}
 
+	// Open the for-statement block scope now, after the range clause.
+	// Iteration variables declared with := need to go in this scope (was issue #51437).
+	check.openScope(s, "range")
+	defer check.closeScope()
+
 	// check assignment to/declaration of iteration variables
 	// (irregular assignment, cannot easily map to existing assignment checks)
 
@@ -874,9 +878,7 @@ func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *s
 	rhs := [2]Type{key, val} // key, val may be nil
 
 	if rclause.Def {
-		// short variable declaration; variable scope starts after the range clause
+		// short variable declaration
-		// (the for loop opens a new scope, so variables on the lhs never redeclare
-		// previously declared variables)
 		var vars []*Var
 		for i, lhs := range lhs {
 			if lhs == nil {
@@ -913,12 +915,8 @@ func (check *Checker) rangeStmt(inner stmtContext, s *syntax.ForStmt, rclause *s
 
 		// declare variables
 		if len(vars) > 0 {
-			scopePos := syntax.EndPos(rclause.X) // TODO(gri) should this just be s.Body.Pos (spec clarification)?
+			scopePos := s.Body.Pos()
 			for _, obj := range vars {
-				// spec: "The scope of a constant or variable identifier declared inside
-				// a function begins at the end of the ConstSpec or VarSpec (ShortVarDecl
-				// for short variable declarations) and ends at the end of the innermost
-				// containing block."
 				check.declare(check.scope, nil /* recordDef already called */, obj, scopePos)
 			}
 		} else {

src/cmd/compile/internal/types2/testdata/fixedbugs/issue51437.go

@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type T struct{}
+
+func (T) m() []int { return nil }
+
+func f(x T) {
+	for _, x := range func() []int {
+		return x.m() // x declared in parameter list of f
+	}() {
+		_ = x // x declared by range clause
+	}
+}

src/go/types/api_test.go

@@ -1690,7 +1690,7 @@ func F(){
 	var F = /*F=func:12*/ F /*F=var:17*/ ; _ = F
 
 	var a []int
-	for i, x := range /*i=undef*/ /*x=var:16*/ a /*i=var:20*/ /*x=var:20*/ { _ = i; _ = x }
+	for i, x := range a /*i=undef*/ /*x=var:16*/ { _ = i; _ = x }
 
 	var i interface{}
 	switch y := i.(type) { /*y=undef*/

src/go/types/stmt.go

@@ -821,8 +821,6 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
 
 	case *ast.RangeStmt:
 		inner |= breakOk | continueOk
-		check.openScope(s, "for")
-		defer check.closeScope()
 
 		// check expression to iterate over
 		var x operand
@@ -857,6 +855,11 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
 			}
 		}
 
+		// Open the for-statement block scope now, after the range clause.
+		// Iteration variables declared with := need to go in this scope (was issue #51437).
+		check.openScope(s, "range")
+		defer check.closeScope()
+
 		// check assignment to/declaration of iteration variables
 		// (irregular assignment, cannot easily map to existing assignment checks)
 
@@ -865,9 +868,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
 		rhs := [2]Type{key, val} // key, val may be nil
 
 		if s.Tok == token.DEFINE {
-			// short variable declaration; variable scope starts after the range clause
+			// short variable declaration
-			// (the for loop opens a new scope, so variables on the lhs never redeclare
-			// previously declared variables)
 			var vars []*Var
 			for i, lhs := range lhs {
 				if lhs == nil {
@@ -904,12 +905,8 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
 
 			// declare variables
 			if len(vars) > 0 {
-				scopePos := s.X.End()
+				scopePos := s.Body.Pos()
 				for _, obj := range vars {
-					// spec: "The scope of a constant or variable identifier declared inside
-					// a function begins at the end of the ConstSpec or VarSpec (ShortVarDecl
-					// for short variable declarations) and ends at the end of the innermost
-					// containing block."
 					check.declare(check.scope, nil /* recordDef already called */, obj, scopePos)
 				}
 			} else {

src/go/types/testdata/fixedbugs/issue51437.go

@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type T struct{}
+
+func (T) m() []int { return nil }
+
+func f(x T) {
+	for _, x := range func() []int {
+		return x.m() // x declared in parameter list of f
+	}() {
+		_ = x // x declared by range clause
+	}
+}

test/fixedbugs/issue51437.go

@@ -0,0 +1,19 @@
+// compile
+
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type T struct{}
+
+func (T) m() []T { return nil }
+
+func f(x T) {
+	for _, x := range func() []T {
+		return x.m()
+	}() {
+		_ = x
+	}
+}