[dev.typeparams] cmd/compile/internal/types2: fixes for all.bash

This CL implements a number of minor fixes that were discovered in
getting -G=3 working for running all.bash.

1. Field tags were handled incorrectly. If a struct type had some
fields with tags, but later fields without tags, the trailing tag-less
fields would all copy the tag of the last tagged field. Fixed by
simply reinitializing `tag` to "" for each field visited.

2. Change the ending of switch case clause scopes from the end of the
last statement to the next "case" token or the switch-ending "}"
token. I don't think this is strictly necessary, but it matches my
intuition about where case-clause scopes end and cmd/compile's current
scoping logic (admittedly influenced by the former).

3. Change select statements to correctly use the scope of each
individual communication clause, instead of the scope of the entire
select statement. This issue appears to be due to the original
go/types code being written to rebind "s" from the *SelectStmt to the
Stmt in the range loop, and then being further asserted to "clause" of
type *CommClause. In most places within the loop body, "clause" was
used, but the rebound "s" identifier was used for the scope
boundaries.

However, in the syntax AST, SelectStmt directly contains a
[]*CommClause (rather than a *BlockStmt, with []Stmt), so no assertion
is necessary and instead of rebinding "s", the range loop was updated
to directly declare "clause".

4. The end position for increment/decrement statements (x++/x--) was
incorrectly calculated. Within the syntax AST, these are represented
as "x += ImplicitOne", and for AssignStmts types2 calculated the end
position as the end position of the RHS operand. But ImplicitOne
doesn't have any position information.

To workaround this, this CL detects ImplicitOne and then computes the
end position of the LHS operand instead, and then adds 2. In practice
this should be correct, though it could be wrong for ill-formatted
statements like "x ++".

Change-Id: I13d4830af39cb3f3b9f0d996672869d3db047ed2
Reviewed-on: https://go-review.googlesource.com/c/go/+/282914
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

1 files changed, 23 insertions, 8 deletions

diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go
index 3463cfdf57..52b9794c10 100644
--- a/src/cmd/compile/internal/types2/stmt.go
+++ b/src/cmd/compile/internal/types2/stmt.go
@@ -156,7 +156,11 @@ func (check *Checker) multipleSelectDefaults(list []*syntax.CommClause) {
 }
 
 func (check *Checker) openScope(node syntax.Node, comment string) {
-	scope := NewScope(check.scope, node.Pos(), endPos(node), comment)
+	check.openScopeUntil(node, endPos(node), comment)
+}
+
+func (check *Checker) openScopeUntil(node syntax.Node, end syntax.Pos, comment string) {
+	scope := NewScope(check.scope, node.Pos(), end, comment)
 	check.recordScope(node, scope)
 	check.scope = scope
 }
@@ -522,7 +526,7 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
 
 		check.multipleSelectDefaults(s.Body)
 
-		for _, clause := range s.Body {
+		for i, clause := range s.Body {
 			if clause == nil {
 				continue // error reported before
 			}
@@ -552,8 +556,11 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
 				check.error(clause.Comm, "select case must be send or receive (possibly with assignment)")
 				continue
 			}
-
-			check.openScope(s, "case")
+			end := s.Rbrace
+			if i+1 < len(s.Body) {
+				end = s.Body[i+1].Pos()
+			}
+			check.openScopeUntil(clause, end, "case")
 			if clause.Comm != nil {
 				check.stmt(inner, clause.Comm)
 			}
@@ -631,14 +638,16 @@ func (check *Checker) switchStmt(inner stmtContext, s *syntax.SwitchStmt) {
 			check.invalidASTf(clause, "incorrect expression switch case")
 			continue
 		}
-		check.caseValues(&x, unpackExpr(clause.Cases), seen)
-		check.openScope(clause, "case")
+		end := s.Rbrace
 		inner := inner
 		if i+1 < len(s.Body) {
+			end = s.Body[i+1].Pos()
 			inner |= fallthroughOk
 		} else {
 			inner |= finalSwitchCase
 		}
+		check.caseValues(&x, unpackExpr(clause.Cases), seen)
+		check.openScopeUntil(clause, end, "case")
 		check.stmtList(inner, clause.Body)
 		check.closeScope()
 	}
@@ -681,15 +690,19 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu
 
 	var lhsVars []*Var                // list of implicitly declared lhs variables
 	seen := make(map[Type]syntax.Pos) // map of seen types to positions
-	for _, clause := range s.Body {
+	for i, clause := range s.Body {
 		if clause == nil {
 			check.invalidASTf(s, "incorrect type switch case")
 			continue
 		}
+		end := s.Rbrace
+		if i+1 < len(s.Body) {
+			end = s.Body[i+1].Pos()
+		}
 		// Check each type in this type switch case.
 		cases := unpackExpr(clause.Cases)
 		T := check.caseTypes(&x, xtyp, cases, seen, false)
-		check.openScope(clause, "case")
+		check.openScopeUntil(clause, end, "case")
 		// If lhs exists, declare a corresponding variable in the case-local scope.
 		if lhs != nil {
 			// spec: "The TypeSwitchGuard may include a short variable declaration.
@@ -701,6 +714,8 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu
 				T = x.typ
 			}
 			obj := NewVar(lhs.Pos(), check.pkg, lhs.Value, T)
+			// TODO(mdempsky): Just use clause.Colon? Why did I even suggest
+			// "at the end of the TypeSwitchCase" in #16794 instead?
 			scopePos := clause.Pos() // for default clause (len(List) == 0)
 			if n := len(cases); n > 0 {
 				scopePos = endPos(cases[n-1])