From 69e7b2bcd61e1d318c51e82a5514b21cbdb2dbc5 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 13 Jun 2024 09:29:22 -0700 Subject: [PATCH] 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 --- src/cmd/compile/internal/types2/stmt.go | 36 ++++++++++--------- src/go/types/stmt.go | 36 +++++++++++-------- .../types/testdata/fixedbugs/issue67962.go | 26 ++++++++++++++ 3 files changed, 67 insertions(+), 31 deletions(-) create mode 100644 src/internal/types/testdata/fixedbugs/issue67962.go diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index f9e17aa6168..58783f47c3c 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 f5cceb8e5f9..215b20160d8 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 00000000000..4dbcd31288c --- /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 + } +}