From c761174d9618ba1458eb0d149eff4d42e2dc92ae Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 13 Jun 2024 13:54:05 -0700 Subject: [PATCH] go/types, types2: cleanup of code handling type switch cases Move logic for type-specific variable type into typeCases function which already does all the relevant work. Add more detailed documentation to typeCases function. Uncomment alernative typeCases function so that it is being type- checked and kept up-to-date. Since it's not (yet) used, the code will not appear in the binary. Follow-up on CL 592555. Change-Id: I6e746503827d512a1dbf7b99b48345c480e61200 Reviewed-on: https://go-review.googlesource.com/c/go/+/592616 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley Reviewed-by: Robert Griesemer Auto-Submit: Robert Griesemer --- src/cmd/compile/internal/types2/stmt.go | 137 ++++++++++++++--------- src/go/types/stmt.go | 139 +++++++++++++++--------- 2 files changed, 168 insertions(+), 108 deletions(-) diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index b598a4f068..b471fb1f34 100644 --- a/src/cmd/compile/internal/types2/stmt.go +++ b/src/cmd/compile/internal/types2/stmt.go @@ -284,10 +284,27 @@ func (check *Checker) isNil(e syntax.Expr) bool { // 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) { +// the operand x corresponding to the type switch expression. If that expression is not +// valid, x must be nil. +// +// switch .(type) { +// case : ... +// ... +// } +// +// caseTypes returns the case-specific type for a variable v introduced through a short +// variable declaration by the type switch: +// +// switch v := .(type) { +// case : // T is the type of in this case +// ... +// } +// +// If there is exactly one type expression, T is the type of that expression. If there +// are multiple type expressions, or if predeclared nil is among the types, the result +// is the type of x. If x is invalid (nil), the result is the invalid type. +func (check *Checker) caseTypes(x *operand, types []syntax.Expr, seen map[Type]syntax.Expr) Type { + var T Type var dummy operand L: for _, e := range types { @@ -322,49 +339,72 @@ L: check.typeAssertion(e, x, T, true) } } - return + + // spec: "In clauses with a case listing exactly one type, the variable has that type; + // otherwise, the variable has the type of the expression in the TypeSwitchGuard. + if len(types) != 1 || T == nil { + T = Typ[Invalid] + if x != nil { + T = x.typ + } + } + + assert(T != nil) + return T } // TODO(gri) Once we are certain that typeHash is correct in all situations, use this version of caseTypes instead. // (Currently it may be possible that different types have identical names and import paths due to ImporterFrom.) -// -// func (check *Checker) caseTypes(x *operand, xtyp *Interface, types []syntax.Expr, seen map[string]syntax.Expr) (T Type) { -// var dummy operand -// L: -// for _, e := range types { -// // The spec allows the value nil instead of a type. -// var hash string -// if check.isNil(e) { -// check.expr(nil, &dummy, e) // run e through expr so we get the usual Info recordings -// T = nil -// hash = "" // avoid collision with a type named nil -// } else { -// T = check.varType(e) -// if !isValid(T) { -// continue L -// } -// hash = typeHash(T, nil) -// } -// // look for duplicate types -// if other := seen[hash]; other != nil { -// // talk about "case" rather than "type" because of nil case -// Ts := "nil" -// if T != nil { -// Ts = TypeString(T, check.qualifier) -// } -// err := check.newError(_DuplicateCase) -// err.addf(e, "duplicate case %s in type switch", Ts) -// err.addf(other, "previous case") -// err.report() -// continue L -// } -// seen[hash] = e -// if T != nil { -// check.typeAssertion(e, x, xtyp, T, true) -// } -// } -// return -// } +func (check *Checker) caseTypes_currently_unused(x *operand, xtyp *Interface, types []syntax.Expr, seen map[string]syntax.Expr) Type { + var T Type + var dummy operand +L: + for _, e := range types { + // The spec allows the value nil instead of a type. + var hash string + if check.isNil(e) { + check.expr(nil, &dummy, e) // run e through expr so we get the usual Info recordings + T = nil + hash = "" // avoid collision with a type named nil + } else { + T = check.varType(e) + if !isValid(T) { + continue L + } + panic("enable typeHash(T, nil)") + // hash = typeHash(T, nil) + } + // look for duplicate types + if other := seen[hash]; other != nil { + // talk about "case" rather than "type" because of nil case + Ts := "nil" + if T != nil { + Ts = TypeString(T, check.qualifier) + } + err := check.newError(DuplicateCase) + err.addf(e, "duplicate case %s in type switch", Ts) + err.addf(other, "previous case") + err.report() + continue L + } + seen[hash] = e + if T != nil { + check.typeAssertion(e, x, T, true) + } + } + + // spec: "In clauses with a case listing exactly one type, the variable has that type; + // otherwise, the variable has the type of the expression in the TypeSwitchGuard. + if len(types) != 1 || T == nil { + T = Typ[Invalid] + if x != nil { + T = x.typ + } + } + + assert(T != nil) + return T +} // stmt typechecks statement s. func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) { @@ -777,17 +817,6 @@ func (check *Checker) typeSwitchStmt(inner stmtContext, s *syntax.SwitchStmt, gu 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. - // When that form is used, the variable is declared at the beginning of - // the implicit block in each clause. In clauses with a case listing - // 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 = 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 // "at the end of the TypeSwitchCase" in go.dev/issue/16794 instead? diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go index c9f7a4f929..74a64f40aa 100644 --- a/src/go/types/stmt.go +++ b/src/go/types/stmt.go @@ -281,10 +281,27 @@ func (check *Checker) isNil(e ast.Expr) bool { // 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) { +// the operand x corresponding to the type switch expression. If that expression is not +// valid, x must be nil. +// +// switch .(type) { +// case : ... +// ... +// } +// +// caseTypes returns the case-specific type for a variable v introduced through a short +// variable declaration by the type switch: +// +// switch v := .(type) { +// case : // T is the type of in this case +// ... +// } +// +// If there is exactly one type expression, T is the type of that expression. If there +// are multiple type expressions, or if predeclared nil is among the types, the result +// is the type of x. If x is invalid (nil), the result is the invalid type. +func (check *Checker) caseTypes(x *operand, types []ast.Expr, seen map[Type]ast.Expr) Type { + var T Type var dummy operand L: for _, e := range types { @@ -319,49 +336,72 @@ L: check.typeAssertion(e, x, T, true) } } - return + + // spec: "In clauses with a case listing exactly one type, the variable has that type; + // otherwise, the variable has the type of the expression in the TypeSwitchGuard. + if len(types) != 1 || T == nil { + T = Typ[Invalid] + if x != nil { + T = x.typ + } + } + + assert(T != nil) + return T } // TODO(gri) Once we are certain that typeHash is correct in all situations, use this version of caseTypes instead. // (Currently it may be possible that different types have identical names and import paths due to ImporterFrom.) -// -// func (check *Checker) caseTypes(x *operand, xtyp *Interface, types []ast.Expr, seen map[string]ast.Expr) (T Type) { -// var dummy operand -// L: -// for _, e := range types { -// // The spec allows the value nil instead of a type. -// var hash string -// if check.isNil(e) { -// check.expr(nil, &dummy, e) // run e through expr so we get the usual Info recordings -// T = nil -// hash = "" // avoid collision with a type named nil -// } else { -// T = check.varType(e) -// if !isValid(T) { -// continue L -// } -// hash = typeHash(T, nil) -// } -// // look for duplicate types -// if other := seen[hash]; other != nil { -// // talk about "case" rather than "type" because of nil case -// Ts := "nil" -// if T != nil { -// Ts = TypeString(T, check.qualifier) -// } -// err := check.newError(_DuplicateCase) -// err.addf(e, "duplicate case %s in type switch", Ts) -// err.addf(other, "previous case") -// err.report() -// continue L -// } -// seen[hash] = e -// if T != nil { -// check.typeAssertion(e.Pos(), x, xtyp, T) -// } -// } -// return -// } +func (check *Checker) caseTypes_currently_unused(x *operand, xtyp *Interface, types []ast.Expr, seen map[string]ast.Expr) Type { + var T Type + var dummy operand +L: + for _, e := range types { + // The spec allows the value nil instead of a type. + var hash string + if check.isNil(e) { + check.expr(nil, &dummy, e) // run e through expr so we get the usual Info recordings + T = nil + hash = "" // avoid collision with a type named nil + } else { + T = check.varType(e) + if !isValid(T) { + continue L + } + panic("enable typeHash(T, nil)") + // hash = typeHash(T, nil) + } + // look for duplicate types + if other := seen[hash]; other != nil { + // talk about "case" rather than "type" because of nil case + Ts := "nil" + if T != nil { + Ts = TypeString(T, check.qualifier) + } + err := check.newError(DuplicateCase) + err.addf(e, "duplicate case %s in type switch", Ts) + err.addf(other, "previous case") + err.report() + continue L + } + seen[hash] = e + if T != nil { + check.typeAssertion(e, x, T, true) + } + } + + // spec: "In clauses with a case listing exactly one type, the variable has that type; + // otherwise, the variable has the type of the expression in the TypeSwitchGuard. + if len(types) != 1 || T == nil { + T = Typ[Invalid] + if x != nil { + T = x.typ + } + } + + assert(T != nil) + return T +} // stmt typechecks statement s. func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) { @@ -722,18 +762,9 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) { check.openScope(clause, "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. - // When that form is used, the variable is declared at the beginning of - // the implicit block in each clause. In clauses with a case listing - // 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 = Typ[Invalid] - if sx != nil { - T = sx.typ - } - } obj := NewVar(lhs.Pos(), check.pkg, lhs.Name, T) + // TODO(mdempsky): Just use clause.Colon? Why did I even suggest + // "at the end of the TypeSwitchCase" in go.dev/issue/16794 instead? scopePos := clause.Pos() + token.Pos(len("default")) // for default clause (len(List) == 0) if n := len(clause.List); n > 0 { scopePos = clause.List[n-1].End() -- 2.48.1