From f6d536bea4efac42c5baee5188293ad7be2f70c3 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 23 Aug 2011 09:22:41 -0700 Subject: [PATCH] go/parser: fix type switch scoping Introduce extra scope for the variable declared by a TypeSwitchGuard so that it doesn't conflict with vars declared by the initial SimpleStmt of a type switch. This is a replacement for CL 4896053 which caused a build breakage. Also: - explicitly detect type switches (as opposed to detecting expression switches and then do extra testing for type switches) - fix all outstanding TODOs in parser.go - ran all tests R=rsc CC=golang-dev https://golang.org/cl/4914044 --- src/cmd/gotype/testdata/test1.go | 17 ++++++++ src/pkg/go/parser/parser.go | 69 +++++++++++++++++++------------- src/pkg/go/parser/parser_test.go | 3 ++ 3 files changed, 61 insertions(+), 28 deletions(-) diff --git a/src/cmd/gotype/testdata/test1.go b/src/cmd/gotype/testdata/test1.go index 0bd46568d6..a3298e6e5f 100644 --- a/src/cmd/gotype/testdata/test1.go +++ b/src/cmd/gotype/testdata/test1.go @@ -4,3 +4,20 @@ func _() { // the scope of a local type declaration starts immediately after the type name type T struct{ _ *T } } + +func _(x interface{}) { + // the variable defined by a TypeSwitchGuard is declared in each TypeCaseClause + switch t := x.(type) { + case int: + _ = t + case float32: + _ = t + default: + _ = t + } + + // the variable defined by a TypeSwitchGuard must not conflict with other + // variables declared in the initial simple statement + switch t := 0; t := x.(type) { + } +} diff --git a/src/pkg/go/parser/parser.go b/src/pkg/go/parser/parser.go index 9c14d16673..7a9ed9dedd 100644 --- a/src/pkg/go/parser/parser.go +++ b/src/pkg/go/parser/parser.go @@ -587,7 +587,6 @@ func (p *parser) parseStructType() *ast.StructType { } rbrace := p.expect(token.RBRACE) - // TODO(gri): store struct scope in AST return &ast.StructType{pos, &ast.FieldList{lbrace, list, rbrace}, false} } @@ -800,7 +799,6 @@ func (p *parser) parseInterfaceType() *ast.InterfaceType { } rbrace := p.expect(token.RBRACE) - // TODO(gri): store interface scope in AST return &ast.InterfaceType{pos, &ast.FieldList{lbrace, list, rbrace}, false} } @@ -1436,14 +1434,14 @@ func (p *parser) parseSimpleStmt(mode int) (ast.Stmt, bool) { case token.ARROW: // send statement arrow := p.pos - p.next() // consume "<-" + p.next() y := p.parseRhs() return &ast.SendStmt{x[0], arrow, y}, false case token.INC, token.DEC: // increment or decrement s := &ast.IncDecStmt{x[0], p.pos, p.tok} - p.next() // consume "++" or "--" + p.next() return s, false } @@ -1591,7 +1589,7 @@ func (p *parser) parseTypeList() (list []ast.Expr) { return } -func (p *parser) parseCaseClause(exprSwitch bool) *ast.CaseClause { +func (p *parser) parseCaseClause(typeSwitch bool) *ast.CaseClause { if p.trace { defer un(trace(p, "CaseClause")) } @@ -1600,10 +1598,10 @@ func (p *parser) parseCaseClause(exprSwitch bool) *ast.CaseClause { var list []ast.Expr if p.tok == token.CASE { p.next() - if exprSwitch { - list = p.parseRhsList() - } else { + if typeSwitch { list = p.parseTypeList() + } else { + list = p.parseRhsList() } } else { p.expect(token.DEFAULT) @@ -1617,15 +1615,19 @@ func (p *parser) parseCaseClause(exprSwitch bool) *ast.CaseClause { return &ast.CaseClause{pos, list, colon, body} } -func isExprSwitch(s ast.Stmt) bool { - if s == nil { - return true - } - if e, ok := s.(*ast.ExprStmt); ok { - if a, ok := e.X.(*ast.TypeAssertExpr); ok { - return a.Type != nil // regular type assertion - } - return true +func isTypeSwitchAssert(x ast.Expr) bool { + a, ok := x.(*ast.TypeAssertExpr) + return ok && a.Type == nil +} + +func isTypeSwitchGuard(s ast.Stmt) bool { + switch t := s.(type) { + case *ast.ExprStmt: + // x.(nil) + return isTypeSwitchAssert(t.X) + case *ast.AssignStmt: + // v := x.(nil) + return len(t.Lhs) == 1 && t.Tok == token.DEFINE && len(t.Rhs) == 1 && isTypeSwitchAssert(t.Rhs[0]) } return false } @@ -1651,28 +1653,41 @@ func (p *parser) parseSwitchStmt() ast.Stmt { s1 = s2 s2 = nil if p.tok != token.LBRACE { + // A TypeSwitchGuard may declare a variable in addition + // to the variable declared in the initial SimpleStmt. + // Introduce extra scope to avoid redeclaration errors: + // + // switch t := 0; t := x.(T) { ... } + // + // (this code is not valid Go because the first t will + // cannot be accessed and thus is never used, the extra + // scope is needed for the correct error message). + // + // If we don't have a type switch, s2 must be an expression. + // Having the extra nested but empty scope won't affect it. + p.openScope() + defer p.closeScope() s2, _ = p.parseSimpleStmt(basic) } } p.exprLev = prevLev } - exprSwitch := isExprSwitch(s2) + typeSwitch := isTypeSwitchGuard(s2) lbrace := p.expect(token.LBRACE) var list []ast.Stmt for p.tok == token.CASE || p.tok == token.DEFAULT { - list = append(list, p.parseCaseClause(exprSwitch)) + list = append(list, p.parseCaseClause(typeSwitch)) } rbrace := p.expect(token.RBRACE) p.expectSemi() body := &ast.BlockStmt{lbrace, list, rbrace} - if exprSwitch { - return &ast.SwitchStmt{pos, s1, p.makeExpr(s2), body} + if typeSwitch { + return &ast.TypeSwitchStmt{pos, s1, s2, body} } - // type switch - // TODO(gri): do all the checks! - return &ast.TypeSwitchStmt{pos, s1, s2, body} + + return &ast.SwitchStmt{pos, s1, p.makeExpr(s2), body} } func (p *parser) parseCommClause() *ast.CommClause { @@ -2001,14 +2016,12 @@ func (p *parser) parseReceiver(scope *ast.Scope) *ast.FieldList { defer un(trace(p, "Receiver")) } - pos := p.pos par := p.parseParameters(scope, false) // must have exactly one receiver if par.NumFields() != 1 { - p.errorExpected(pos, "exactly one receiver") - // TODO determine a better range for BadExpr below - par.List = []*ast.Field{&ast.Field{Type: &ast.BadExpr{pos, pos}}} + p.errorExpected(par.Opening, "exactly one receiver") + par.List = []*ast.Field{&ast.Field{Type: &ast.BadExpr{par.Opening, par.Closing + 1}}} return par } diff --git a/src/pkg/go/parser/parser_test.go b/src/pkg/go/parser/parser_test.go index 39a78e5156..fb91dd1e7c 100644 --- a/src/pkg/go/parser/parser_test.go +++ b/src/pkg/go/parser/parser_test.go @@ -26,6 +26,9 @@ var illegalInputs = []interface{}{ `package p; func f() { for _ = range x ; ; {} };`, `package p; func f() { for ; ; _ = range x {} };`, `package p; func f() { for ; _ = range x ; {} };`, + `package p; func f() { switch t = t.(type) {} };`, + `package p; func f() { switch t, t = t.(type) {} };`, + `package p; func f() { switch t = t.(type), t {} };`, `package p; var a = [1]int; /* illegal expression */`, `package p; var a = [...]int; /* illegal expression */`, `package p; var a = struct{} /* illegal expression */`, -- 2.50.0