mirror of
https://github.com/golang/go
synced 2024-09-15 22:20:06 +00:00
cmd/compile: better handling of incorrect type switches
Don't report errors if we don't have a correct type switch guard; instead ignore it and leave it to the type-checker to report the error. This leads to better error messages concentrating on the type switch guard rather than errors around (confusing) syntactic details. Also clean up some code setting up AssertExpr (they never have a nil Type field) and remove some incorrect TODOs. Fixes #24470. Change-Id: I69512f36e0417e3b5ea9c8856768e04b19d654a8 Reviewed-on: https://go-review.googlesource.com/103615 Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This commit is contained in:
parent
071f0de4ec
commit
c65a2781be
|
@ -598,13 +598,7 @@ func (p *noder) expr(expr syntax.Expr) *Node {
|
||||||
n.SetSliceBounds(index[0], index[1], index[2])
|
n.SetSliceBounds(index[0], index[1], index[2])
|
||||||
return n
|
return n
|
||||||
case *syntax.AssertExpr:
|
case *syntax.AssertExpr:
|
||||||
if expr.Type == nil {
|
return p.nod(expr, ODOTTYPE, p.expr(expr.X), p.typeExpr(expr.Type))
|
||||||
panic("unexpected AssertExpr")
|
|
||||||
}
|
|
||||||
// TODO(mdempsky): parser.pexpr uses p.expr(), but
|
|
||||||
// seems like the type field should be parsed with
|
|
||||||
// ntype? Shrug, doesn't matter here.
|
|
||||||
return p.nod(expr, ODOTTYPE, p.expr(expr.X), p.expr(expr.Type))
|
|
||||||
case *syntax.Operation:
|
case *syntax.Operation:
|
||||||
if expr.Op == syntax.Add && expr.Y != nil {
|
if expr.Op == syntax.Add && expr.Y != nil {
|
||||||
return p.sum(expr)
|
return p.sum(expr)
|
||||||
|
|
|
@ -199,11 +199,18 @@ type (
|
||||||
// X.(Type)
|
// X.(Type)
|
||||||
AssertExpr struct {
|
AssertExpr struct {
|
||||||
X Expr
|
X Expr
|
||||||
// TODO(gri) consider using Name{"..."} instead of nil (permits attaching of comments)
|
|
||||||
Type Expr
|
Type Expr
|
||||||
expr
|
expr
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// X.(type)
|
||||||
|
// Lhs := X.(type)
|
||||||
|
TypeSwitchGuard struct {
|
||||||
|
Lhs *Name // nil means no Lhs :=
|
||||||
|
X Expr // X.(type)
|
||||||
|
expr
|
||||||
|
}
|
||||||
|
|
||||||
Operation struct {
|
Operation struct {
|
||||||
Op Operator
|
Op Operator
|
||||||
X, Y Expr // Y == nil means unary expression
|
X, Y Expr // Y == nil means unary expression
|
||||||
|
@ -413,13 +420,6 @@ type (
|
||||||
simpleStmt
|
simpleStmt
|
||||||
}
|
}
|
||||||
|
|
||||||
TypeSwitchGuard struct {
|
|
||||||
// TODO(gri) consider using Name{"..."} instead of nil (permits attaching of comments)
|
|
||||||
Lhs *Name // nil means no Lhs :=
|
|
||||||
X Expr // X.(type)
|
|
||||||
expr
|
|
||||||
}
|
|
||||||
|
|
||||||
CaseClause struct {
|
CaseClause struct {
|
||||||
Cases Expr // nil means default clause
|
Cases Expr // nil means default clause
|
||||||
Body []Stmt
|
Body []Stmt
|
||||||
|
|
|
@ -907,6 +907,7 @@ loop:
|
||||||
p.next()
|
p.next()
|
||||||
if p.got(_Type) {
|
if p.got(_Type) {
|
||||||
t := new(TypeSwitchGuard)
|
t := new(TypeSwitchGuard)
|
||||||
|
// t.Lhs is filled in by parser.simpleStmt
|
||||||
t.pos = pos
|
t.pos = pos
|
||||||
t.X = x
|
t.X = x
|
||||||
x = t
|
x = t
|
||||||
|
@ -914,7 +915,7 @@ loop:
|
||||||
t := new(AssertExpr)
|
t := new(AssertExpr)
|
||||||
t.pos = pos
|
t.pos = pos
|
||||||
t.X = x
|
t.X = x
|
||||||
t.Type = p.expr()
|
t.Type = p.type_()
|
||||||
x = t
|
x = t
|
||||||
}
|
}
|
||||||
p.want(_Rparen)
|
p.want(_Rparen)
|
||||||
|
@ -1584,12 +1585,12 @@ func (p *parser) bad() *BadExpr {
|
||||||
var ImplicitOne = &BasicLit{Value: "1"}
|
var ImplicitOne = &BasicLit{Value: "1"}
|
||||||
|
|
||||||
// SimpleStmt = EmptyStmt | ExpressionStmt | SendStmt | IncDecStmt | Assignment | ShortVarDecl .
|
// SimpleStmt = EmptyStmt | ExpressionStmt | SendStmt | IncDecStmt | Assignment | ShortVarDecl .
|
||||||
func (p *parser) simpleStmt(lhs Expr, rangeOk bool) SimpleStmt {
|
func (p *parser) simpleStmt(lhs Expr, keyword token) SimpleStmt {
|
||||||
if trace {
|
if trace {
|
||||||
defer p.trace("simpleStmt")()
|
defer p.trace("simpleStmt")()
|
||||||
}
|
}
|
||||||
|
|
||||||
if rangeOk && p.tok == _Range {
|
if keyword == _For && p.tok == _Range {
|
||||||
// _Range expr
|
// _Range expr
|
||||||
if debug && lhs != nil {
|
if debug && lhs != nil {
|
||||||
panic("invalid call of simpleStmt")
|
panic("invalid call of simpleStmt")
|
||||||
|
@ -1636,51 +1637,35 @@ func (p *parser) simpleStmt(lhs Expr, rangeOk bool) SimpleStmt {
|
||||||
}
|
}
|
||||||
|
|
||||||
// expr_list
|
// expr_list
|
||||||
pos := p.pos()
|
|
||||||
switch p.tok {
|
switch p.tok {
|
||||||
case _Assign:
|
case _Assign, _Define:
|
||||||
|
pos := p.pos()
|
||||||
|
var op Operator
|
||||||
|
if p.tok == _Define {
|
||||||
|
op = Def
|
||||||
|
}
|
||||||
p.next()
|
p.next()
|
||||||
|
|
||||||
if rangeOk && p.tok == _Range {
|
if keyword == _For && p.tok == _Range {
|
||||||
// expr_list '=' _Range expr
|
// expr_list op= _Range expr
|
||||||
return p.newRangeClause(lhs, false)
|
return p.newRangeClause(lhs, op == Def)
|
||||||
}
|
}
|
||||||
|
|
||||||
// expr_list '=' expr_list
|
// expr_list op= expr_list
|
||||||
return p.newAssignStmt(pos, 0, lhs, p.exprList())
|
|
||||||
|
|
||||||
case _Define:
|
|
||||||
p.next()
|
|
||||||
|
|
||||||
if rangeOk && p.tok == _Range {
|
|
||||||
// expr_list ':=' range expr
|
|
||||||
return p.newRangeClause(lhs, true)
|
|
||||||
}
|
|
||||||
|
|
||||||
// expr_list ':=' expr_list
|
|
||||||
rhs := p.exprList()
|
rhs := p.exprList()
|
||||||
|
|
||||||
if x, ok := rhs.(*TypeSwitchGuard); ok {
|
if x, ok := rhs.(*TypeSwitchGuard); ok && keyword == _Switch && op == Def {
|
||||||
switch lhs := lhs.(type) {
|
if lhs, ok := lhs.(*Name); ok {
|
||||||
case *Name:
|
// switch … lhs := rhs.(type)
|
||||||
x.Lhs = lhs
|
x.Lhs = lhs
|
||||||
case *ListExpr:
|
|
||||||
p.errorAt(lhs.Pos(), fmt.Sprintf("cannot assign 1 value to %d variables", len(lhs.ElemList)))
|
|
||||||
// make the best of what we have
|
|
||||||
if lhs, ok := lhs.ElemList[0].(*Name); ok {
|
|
||||||
x.Lhs = lhs
|
|
||||||
}
|
|
||||||
default:
|
|
||||||
p.errorAt(lhs.Pos(), fmt.Sprintf("invalid variable name %s in type switch", String(lhs)))
|
|
||||||
}
|
|
||||||
s := new(ExprStmt)
|
s := new(ExprStmt)
|
||||||
s.pos = x.Pos()
|
s.pos = x.Pos()
|
||||||
s.X = x
|
s.X = x
|
||||||
return s
|
return s
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
as := p.newAssignStmt(pos, Def, lhs, rhs)
|
return p.newAssignStmt(pos, op, lhs, rhs)
|
||||||
return as
|
|
||||||
|
|
||||||
default:
|
default:
|
||||||
p.syntaxError("expecting := or = or comma")
|
p.syntaxError("expecting := or = or comma")
|
||||||
|
@ -1820,7 +1805,7 @@ func (p *parser) header(keyword token) (init SimpleStmt, cond Expr, post SimpleS
|
||||||
if p.got(_Var) {
|
if p.got(_Var) {
|
||||||
p.syntaxError(fmt.Sprintf("var declaration not allowed in %s initializer", keyword.String()))
|
p.syntaxError(fmt.Sprintf("var declaration not allowed in %s initializer", keyword.String()))
|
||||||
}
|
}
|
||||||
init = p.simpleStmt(nil, keyword == _For)
|
init = p.simpleStmt(nil, keyword)
|
||||||
// If we have a range clause, we are done (can only happen for keyword == _For).
|
// If we have a range clause, we are done (can only happen for keyword == _For).
|
||||||
if _, ok := init.(*RangeClause); ok {
|
if _, ok := init.(*RangeClause); ok {
|
||||||
p.xnest = outer
|
p.xnest = outer
|
||||||
|
@ -1847,17 +1832,17 @@ func (p *parser) header(keyword token) (init SimpleStmt, cond Expr, post SimpleS
|
||||||
p.syntaxError("expecting for loop condition")
|
p.syntaxError("expecting for loop condition")
|
||||||
goto done
|
goto done
|
||||||
}
|
}
|
||||||
condStmt = p.simpleStmt(nil, false)
|
condStmt = p.simpleStmt(nil, 0 /* range not permitted */)
|
||||||
}
|
}
|
||||||
p.want(_Semi)
|
p.want(_Semi)
|
||||||
if p.tok != _Lbrace {
|
if p.tok != _Lbrace {
|
||||||
post = p.simpleStmt(nil, false)
|
post = p.simpleStmt(nil, 0 /* range not permitted */)
|
||||||
if a, _ := post.(*AssignStmt); a != nil && a.Op == Def {
|
if a, _ := post.(*AssignStmt); a != nil && a.Op == Def {
|
||||||
p.syntaxErrorAt(a.Pos(), "cannot declare in post statement of for loop")
|
p.syntaxErrorAt(a.Pos(), "cannot declare in post statement of for loop")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if p.tok != _Lbrace {
|
} else if p.tok != _Lbrace {
|
||||||
condStmt = p.simpleStmt(nil, false)
|
condStmt = p.simpleStmt(nil, keyword)
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
condStmt = init
|
condStmt = init
|
||||||
|
@ -2003,7 +1988,7 @@ func (p *parser) commClause() *CommClause {
|
||||||
switch p.tok {
|
switch p.tok {
|
||||||
case _Case:
|
case _Case:
|
||||||
p.next()
|
p.next()
|
||||||
c.Comm = p.simpleStmt(nil, false)
|
c.Comm = p.simpleStmt(nil, 0)
|
||||||
|
|
||||||
// The syntax restricts the possible simple statements here to:
|
// The syntax restricts the possible simple statements here to:
|
||||||
//
|
//
|
||||||
|
@ -2049,7 +2034,7 @@ func (p *parser) stmtOrNil() Stmt {
|
||||||
if label, ok := lhs.(*Name); ok && p.tok == _Colon {
|
if label, ok := lhs.(*Name); ok && p.tok == _Colon {
|
||||||
return p.labeledStmtOrNil(label)
|
return p.labeledStmtOrNil(label)
|
||||||
}
|
}
|
||||||
return p.simpleStmt(lhs, false)
|
return p.simpleStmt(lhs, 0)
|
||||||
}
|
}
|
||||||
|
|
||||||
switch p.tok {
|
switch p.tok {
|
||||||
|
@ -2068,13 +2053,13 @@ func (p *parser) stmtOrNil() Stmt {
|
||||||
case _Operator, _Star:
|
case _Operator, _Star:
|
||||||
switch p.op {
|
switch p.op {
|
||||||
case Add, Sub, Mul, And, Xor, Not:
|
case Add, Sub, Mul, And, Xor, Not:
|
||||||
return p.simpleStmt(nil, false) // unary operators
|
return p.simpleStmt(nil, 0) // unary operators
|
||||||
}
|
}
|
||||||
|
|
||||||
case _Literal, _Func, _Lparen, // operands
|
case _Literal, _Func, _Lparen, // operands
|
||||||
_Lbrack, _Struct, _Map, _Chan, _Interface, // composite types
|
_Lbrack, _Struct, _Map, _Chan, _Interface, // composite types
|
||||||
_Arrow: // receive operator
|
_Arrow: // receive operator
|
||||||
return p.simpleStmt(nil, false)
|
return p.simpleStmt(nil, 0)
|
||||||
|
|
||||||
case _For:
|
case _For:
|
||||||
return p.forStmt()
|
return p.forStmt()
|
||||||
|
|
|
@ -393,13 +393,13 @@ func (p *printer) printRawNode(n Node) {
|
||||||
p.print(_Rbrack)
|
p.print(_Rbrack)
|
||||||
|
|
||||||
case *AssertExpr:
|
case *AssertExpr:
|
||||||
p.print(n.X, _Dot, _Lparen)
|
p.print(n.X, _Dot, _Lparen, n.Type, _Rparen)
|
||||||
if n.Type != nil {
|
|
||||||
p.printNode(n.Type)
|
case *TypeSwitchGuard:
|
||||||
} else {
|
if n.Lhs != nil {
|
||||||
p.print(_Type)
|
p.print(n.Lhs, blank, _Define, blank)
|
||||||
}
|
}
|
||||||
p.print(_Rparen)
|
p.print(n.X, _Dot, _Lparen, _Type, _Rparen)
|
||||||
|
|
||||||
case *CallExpr:
|
case *CallExpr:
|
||||||
p.print(n.Fun, _Lparen)
|
p.print(n.Fun, _Lparen)
|
||||||
|
@ -557,12 +557,6 @@ func (p *printer) printRawNode(n Node) {
|
||||||
}
|
}
|
||||||
p.printSwitchBody(n.Body)
|
p.printSwitchBody(n.Body)
|
||||||
|
|
||||||
case *TypeSwitchGuard:
|
|
||||||
if n.Lhs != nil {
|
|
||||||
p.print(n.Lhs, blank, _Define, blank)
|
|
||||||
}
|
|
||||||
p.print(n.X, _Dot, _Lparen, _Type, _Rparen)
|
|
||||||
|
|
||||||
case *SelectStmt:
|
case *SelectStmt:
|
||||||
p.print(_Select, blank) // for now
|
p.print(_Select, blank) // for now
|
||||||
p.printSelectBody(n.Body)
|
p.printSelectBody(n.Body)
|
||||||
|
|
15
test/fixedbugs/issue24470.go
Normal file
15
test/fixedbugs/issue24470.go
Normal file
|
@ -0,0 +1,15 @@
|
||||||
|
// errorcheck
|
||||||
|
|
||||||
|
// Copyright 2018 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.
|
||||||
|
|
||||||
|
// Verify that we get "use of .(type) outside type switch"
|
||||||
|
// before any other (misleading) errors. Test case from issue.
|
||||||
|
|
||||||
|
package p
|
||||||
|
|
||||||
|
func f(i interface{}) {
|
||||||
|
if x, ok := i.(type); ok { // ERROR "outside type switch"
|
||||||
|
}
|
||||||
|
}
|
|
@ -7,7 +7,7 @@
|
||||||
package main
|
package main
|
||||||
|
|
||||||
func main() {
|
func main() {
|
||||||
switch main() := interface{}(nil).(type) { // ERROR "invalid variable name"
|
switch main() := interface{}(nil).(type) { // ERROR "invalid variable name|used as value"
|
||||||
default:
|
default:
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue