[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>

diff --git a/src/cmd/compile/internal/types2/api_test.go b/src/cmd/compile/internal/types2/api_test.go
index d9647b943247e78b79afdf52c27dd7151dd08f1c..81fc1243e96a2114bf4934b78b6b887b37e48753 100644 (file)
--- a/src/cmd/compile/internal/types2/api_test.go
+++ b/src/cmd/compile/internal/types2/api_test.go
@@ -799,10 +799,10 @@ func TestScopesInfo(t *testing.T) {
                        "file:", "func:",
                }},
                {`package p15; func _(c chan int) { select{ case <-c: } }`, []string{
-                       "file:", "func:c", "select:",
+                       "file:", "func:c", "comm:",
                }},
                {`package p16; func _(c chan int) { select{ case i := <-c: x := i; _ = x} }`, []string{
-                       "file:", "func:c", "select:i x",
+                       "file:", "func:c", "comm:i x",
                }},
                {`package p17; func _() { for{} }`, []string{
                        "file:", "func:", "for:", "block:",

diff --git a/src/cmd/compile/internal/types2/pos.go b/src/cmd/compile/internal/types2/pos.go
index 4dd839b7dced1fb5f852def233d638a9b176ff1b..0a19cd1a239a064e29e268bb9b72b12dcbce68e0 100644 (file)
--- a/src/cmd/compile/internal/types2/pos.go
+++ b/src/cmd/compile/internal/types2/pos.go
@@ -286,6 +286,10 @@ func endPos(n syntax.Node) syntax.Pos {
                        return n.Pos()
                case *syntax.AssignStmt:
                        m = n.Rhs
+                       if m == syntax.ImplicitOne {
+                               p := endPos(n.Lhs)
+                               return syntax.MakePos(p.Base(), p.Line(), p.Col()+2)
+                       }
                case *syntax.BranchStmt:
                        if n.Label != nil {
                                m = n.Label

diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go
index 3463cfdf57a0534e085060f0f9a41e4bcfe33018..52b9794c102f17f799d600cd87d7befa2b5c6f2b 100644 (file)
--- 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])

diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go
index 910db0819f75eb152dc9aced11528dbca8fb3aa9..32377ed3f457309d972147847251606c50f89eca 100644 (file)
--- a/src/cmd/compile/internal/types2/typexpr.go
+++ b/src/cmd/compile/internal/types2/typexpr.go
@@ -1110,6 +1110,7 @@ func (check *Checker) structType(styp *Struct, e *syntax.StructType) {
                        typ = check.varType(f.Type)
                        prev = f.Type
                }
+               tag = ""
                if i < len(e.TagList) {
                        tag = check.tag(e.TagList[i])
                }