From: Robert Griesemer Date: Thu, 13 Jun 2024 16:29:22 +0000 (-0700) Subject: go/types, types2: typecheck cases even if switch expression is invalid X-Git-Tag: go1.23rc1~26 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=69e7b2bcd61e1d318c51e82a5514b21cbdb2dbc5;p=gostls13.git go/types, types2: typecheck cases even if switch expression is invalid Rather than returning right away when the switch expression is invalid, continue type checking the type switch case. The code was already written to be able to deal with an invalid switch expression but it returned early nevertheless. Remove the early return and rewrite the switch expression test slightly to better control the scope of the x operand, leading to cleaner code. In the process replace a tricky use of the x operand with a use of the sx operand (plus guard, since sx may be nil if invalid). Fixes #67962. Change-Id: I1dc08d10078753c68449637622beb4018ed23803 Reviewed-on: https://go-review.googlesource.com/c/go/+/592555 Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Griesemer Auto-Submit: Robert Griesemer --- diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index f9e17aa616..58783f47c3 100644 --- a/src/cmd/compile/internal/types2/stmt.go +++ b/src/cmd/compile/internal/types2/stmt.go @@ -282,7 +282,11 @@ func (check *Checker) isNil(e syntax.Expr) bool { return false } -// If the type switch expression is invalid, x is nil. +// caseTypes typechecks the type expressions of a type case, checks for duplicate types +// using the seen map, and verifies that each type is valid with respect to the type of +// the operand x in the type switch clause. If the type switch expression is invalid, x +// must be nil. The result is the type of the last type expression; it is nil if the +// expression denotes the predeclared nil. func (check *Checker) caseTypes(x *operand, types []syntax.Expr, seen map[Type]syntax.Expr) (T Type) { var dummy operand L: @@ -739,21 +743,18 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu } // check rhs - var x operand - check.expr(nil, &x, guard.X) - if x.mode == invalid { - return - } - - // TODO(gri) we may want to permit type switches on type parameter values at some point var sx *operand // switch expression against which cases are compared against; nil if invalid - if isTypeParam(x.typ) { - check.errorf(&x, InvalidTypeSwitch, "cannot use type switch on type parameter value %s", &x) - } else { - if _, ok := under(x.typ).(*Interface); ok { - sx = &x - } else { - check.errorf(&x, InvalidTypeSwitch, "%s is not an interface", &x) + { + var x operand + check.expr(nil, &x, guard.X) + if x.mode != invalid { + if isTypeParam(x.typ) { + check.errorf(&x, InvalidTypeSwitch, "cannot use type switch on type parameter value %s", &x) + } else if IsInterface(x.typ) { + sx = &x + } else { + check.errorf(&x, InvalidTypeSwitch, "%s is not an interface", &x) + } } } @@ -782,7 +783,10 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu // exactly one type, the variable has that type; otherwise, the variable // has the type of the expression in the TypeSwitchGuard." if len(cases) != 1 || T == nil { - T = x.typ + T = Typ[Invalid] + if sx != nil { + T = sx.typ + } } obj := NewVar(lhs.Pos(), check.pkg, lhs.Value, T) // TODO(mdempsky): Just use clause.Colon? Why did I even suggest diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go index f5cceb8e5f..215b20160d 100644 --- a/src/go/types/stmt.go +++ b/src/go/types/stmt.go @@ -279,7 +279,11 @@ func (check *Checker) isNil(e ast.Expr) bool { return false } -// If the type switch expression is invalid, x is nil. +// caseTypes typechecks the type expressions of a type case, checks for duplicate types +// using the seen map, and verifies that each type is valid with respect to the type of +// the operand x in the type switch clause. If the type switch expression is invalid, x +// must be nil. The result is the type of the last type expression; it is nil if the +// expression denotes the predeclared nil. func (check *Checker) caseTypes(x *operand, types []ast.Expr, seen map[Type]ast.Expr) (T Type) { var dummy operand L: @@ -687,20 +691,19 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) { check.error(s, InvalidSyntaxTree, "incorrect form of type switch guard") return } - var x operand - check.expr(nil, &x, expr.X) - if x.mode == invalid { - return - } - // TODO(gri) we may want to permit type switches on type parameter values at some point + var sx *operand // switch expression against which cases are compared against; nil if invalid - if isTypeParam(x.typ) { - check.errorf(&x, InvalidTypeSwitch, "cannot use type switch on type parameter value %s", &x) - } else { - if _, ok := under(x.typ).(*Interface); ok { - sx = &x - } else { - check.errorf(&x, InvalidTypeSwitch, "%s is not an interface", &x) + { + var x operand + check.expr(nil, &x, expr.X) + if x.mode != invalid { + if isTypeParam(x.typ) { + check.errorf(&x, InvalidTypeSwitch, "cannot use type switch on type parameter value %s", &x) + } else if IsInterface(x.typ) { + sx = &x + } else { + check.errorf(&x, InvalidTypeSwitch, "%s is not an interface", &x) + } } } @@ -725,7 +728,10 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) { // exactly one type, the variable has that type; otherwise, the variable // has the type of the expression in the TypeSwitchGuard." if len(clause.List) != 1 || T == nil { - T = x.typ + T = Typ[Invalid] + if sx != nil { + T = sx.typ + } } obj := NewVar(lhs.Pos(), check.pkg, lhs.Name, T) scopePos := clause.Pos() + token.Pos(len("default")) // for default clause (len(List) == 0) diff --git a/src/internal/types/testdata/fixedbugs/issue67962.go b/src/internal/types/testdata/fixedbugs/issue67962.go new file mode 100644 index 0000000000..4dbcd31288 --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue67962.go @@ -0,0 +1,26 @@ +// Copyright 2024 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. + +package p + +// No `"fmt" imported and not used` error below. +// The switch cases must be typechecked even +// though the switch expression is invalid. + +import "fmt" + +func _() { + x := 1 + for e := range x.m /* ERROR "x.m undefined (type int has no field or method m)" */ () { + switch e.(type) { + case int: + fmt.Println() + } + } + + switch t := x /* ERROR "not an interface" */ .(type) { + case int, string: + _ = t + } +}