From a4c5198a3c8befa2f126fd365de4dc09c32b7886 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 18 Jul 2022 12:28:11 -0700 Subject: [PATCH] [dev.unified] cmd/compile/internal/noder: better switch statements Walk desugars switch statements into a bunch of OEQ comparisons, and sometimes (although rarely in practice) this currently requires converting the tag value to the case value's type. And because this conversion is inserted during walk, unified IR can't wire up appropriate RTTI operands for the conversion. As a simple solution, if any of the case values are *not* assignable to the tag value's type, we instead convert them all to `any`. This works because `any(x) == any(y)` yields the same value as `x == y`, as long as neither `x` nor `y` are `nil`. We never have to worry about `x` or `y` being `nil` either, because: 1. `switch nil` is invalid, so `x` can never be `nil`. 2. If the tag type is a channel, map, or function type, they can *only* be compared against `nil`; so the case values will always be assignable to the tag value's type, and so we won't convert to `any`. 3. For other nullable types, the previous commit (adding explicit `nil` handling to unified IR) ensures that `case nil:` is actually treated as `case tagType(nil):`. Change-Id: I3adcb9cf0d42a91a12b1a163c58d4133a24fca5e Reviewed-on: https://go-review.googlesource.com/c/go/+/418101 Reviewed-by: Keith Randall Reviewed-by: Cuong Manh Le Run-TryBot: Matthew Dempsky TryBot-Result: Gopher Robot Reviewed-by: Keith Randall --- src/cmd/compile/internal/noder/writer.go | 42 +++++++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/cmd/compile/internal/noder/writer.go b/src/cmd/compile/internal/noder/writer.go index 47384c6c64065..09afbcdffbfb8 100644 --- a/src/cmd/compile/internal/noder/writer.go +++ b/src/cmd/compile/internal/noder/writer.go @@ -1311,7 +1311,7 @@ func (w *writer) switchStmt(stmt *syntax.SwitchStmt) { w.pos(stmt) w.stmt(stmt.Init) - var iface types2.Type + var iface, tagType types2.Type if guard, ok := stmt.Tag.(*syntax.TypeSwitchGuard); w.Bool(ok) { iface = w.p.typeOf(guard.X) @@ -1322,7 +1322,32 @@ func (w *writer) switchStmt(stmt *syntax.SwitchStmt) { } w.expr(guard.X) } else { - w.optExpr(stmt.Tag) + tag := stmt.Tag + + if tag != nil { + tagType = w.p.typeOf(tag) + } else { + tagType = types2.Typ[types2.Bool] + } + + // Walk is going to emit comparisons between the tag value and + // each case expression, and we want these comparisons to always + // have the same type. If there are any case values that can't be + // converted to the tag value's type, then convert everything to + // `any` instead. + Outer: + for _, clause := range stmt.Body { + for _, cas := range unpackListExpr(clause.Cases) { + if casType := w.p.typeOf(cas); !types2.AssignableTo(casType, tagType) { + tagType = types2.NewInterfaceType(nil, nil) + break Outer + } + } + } + + if w.Bool(tag != nil) { + w.implicitConvExpr(tag, tagType, tag) + } } w.Len(len(stmt.Body)) @@ -1334,15 +1359,22 @@ func (w *writer) switchStmt(stmt *syntax.SwitchStmt) { w.pos(clause) + cases := unpackListExpr(clause.Cases) if iface != nil { - cases := unpackListExpr(clause.Cases) w.Len(len(cases)) for _, cas := range cases { w.exprType(iface, cas, true) } } else { - // TODO(mdempsky): Implicit conversions to tagType, if appropriate. - w.exprList(clause.Cases) + // As if w.exprList(clause.Cases), + // but with implicit conversions to tagType. + + w.Sync(pkgbits.SyncExprList) + w.Sync(pkgbits.SyncExprs) + w.Len(len(cases)) + for _, cas := range cases { + w.implicitConvExpr(cas, tagType, cas) + } } if obj, ok := w.p.info.Implicits[clause]; ok {