mirror of
https://github.com/golang/go
synced 2024-11-22 16:34:47 -07:00
[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>
This commit is contained in:
parent
8123bc90b8
commit
44d1a8523a
@ -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:",
|
||||
|
@ -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
|
||||
|
@ -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])
|
||||
|
@ -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])
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user