cmd/compile: make duplicate expr cases readable

Instead of just printing the value, print the original node to make the
error more human-friendly. Also print the value if its string form is
different than the original node, to make sure it's obvious what value
was duplicated.

This means that "case '@', '@':", which used to print:

	duplicate case 64 in switch

Will now print:

	duplicate case '@' (value 64) in switch

Factor this logic out into its own function to reuse it in range cases
and any other place where we might want to print a node and its value in
the future.

Also needed to split the errorcheck files because expression switch case
duplicates are now detected earlier, so they stop the compiler before it
gets to generating the AST and detecting the type switch case
duplicates.

Fixes #20112.

Change-Id: I9009b50dec0d0e705e5de9c9ccb08f1dce8a5a99
Reviewed-on: https://go-review.googlesource.com/41852
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

1 files changed, 42 insertions, 48 deletions

diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go
index 871cb5b8b1..1b76650a7f 100644
--- a/src/cmd/compile/internal/gc/swt.go
+++ b/src/cmd/compile/internal/gc/swt.go
@@ -6,6 +6,7 @@ package gc
 
 import (
 	"cmd/compile/internal/types"
+	"fmt"
 	"sort"
 )
 
@@ -203,6 +204,11 @@ func typecheckswitch(n *Node) {
 
 		typecheckslice(ncase.Nbody.Slice(), Etop)
 	}
+	switch top {
+	// expression switch
+	case Erv:
+		checkDupExprCases(n.Left, n.List.Slice())
+	}
 }
 
 // walkswitch walks a switch statement.
@@ -523,9 +529,6 @@ func (s *exprSwitch) genCaseClauses(clauses []*Node) caseClauses {
 	if cc.defjmp == nil {
 		cc.defjmp = nod(OBREAK, nil, nil)
 	}
-
-	// diagnose duplicate cases
-	s.checkDupCases(cc.list)
 	return cc
 }
 
@@ -599,20 +602,18 @@ Outer:
 	}
 }
 
-func (s *exprSwitch) checkDupCases(cc []caseClause) {
-	if len(cc) < 2 {
+func checkDupExprCases(exprname *Node, clauses []*Node) {
+	// boolean (naked) switch, nothing to do.
+	if exprname == nil {
 		return
 	}
 	// The common case is that s's expression is not an interface.
 	// In that case, all constant clauses have the same type,
 	// so checking for duplicates can be done solely by value.
-	if !s.exprname.Type.IsInterface() {
+	if !exprname.Type.IsInterface() {
 		seen := make(map[interface{}]*Node)
-		for _, c := range cc {
-			switch {
-			case c.node.Left != nil:
-				// Single constant.
-
+		for _, ncase := range clauses {
+			for _, n := range ncase.List.Slice() {
 				// Can't check for duplicates that aren't constants, per the spec. Issue 15896.
 				// Don't check for duplicate bools. Although the spec allows it,
 				// (1) the compiler hasn't checked it in the past, so compatibility mandates it, and
@@ -620,35 +621,18 @@ func (s *exprSwitch) checkDupCases(cc []caseClause) {
 				//       case GOARCH == "arm" && GOARM == "5":
 				//       case GOARCH == "arm":
 				//     which would both evaluate to false for non-ARM compiles.
-				if ct := consttype(c.node.Left); ct < 0 || ct == CTBOOL {
+				if ct := consttype(n); ct < 0 || ct == CTBOOL {
 					continue
 				}
 
-				val := c.node.Left.Val().Interface()
+				val := n.Val().Interface()
 				prev, dup := seen[val]
 				if !dup {
-					seen[val] = c.node
+					seen[val] = n
 					continue
 				}
-				setlineno(c.node)
-				yyerror("duplicate case %#v in switch\n\tprevious case at %v", val, prev.Line())
-
-			case c.node.List.Len() == 2:
-				// Range of integers.
-				low := c.node.List.First().Int64()
-				high := c.node.List.Second().Int64()
-				for i := low; i <= high; i++ {
-					prev, dup := seen[i]
-					if !dup {
-						seen[i] = c.node
-						continue
-					}
-					setlineno(c.node)
-					yyerror("duplicate case %d in switch\n\tprevious case at %v", i, prev.Line())
-				}
-
-			default:
-				Fatalf("bad caseClause node in checkDupCases: %v", c.node)
+				yyerrorl(ncase.Pos, "duplicate case %s in switch\n\tprevious case at %v",
+					nodeAndVal(n), prev.Line())
 			}
 		}
 		return
@@ -660,25 +644,35 @@ func (s *exprSwitch) checkDupCases(cc []caseClause) {
 		val interface{}
 	}
 	seen := make(map[typeVal]*Node)
-	for _, c := range cc {
-		if ct := consttype(c.node.Left); ct < 0 || ct == CTBOOL {
-			continue
-		}
-		n := c.node.Left
-		tv := typeVal{
-			typ: n.Type.LongString(),
-			val: n.Val().Interface(),
-		}
-		prev, dup := seen[tv]
-		if !dup {
-			seen[tv] = c.node
-			continue
+	for _, ncase := range clauses {
+		for _, n := range ncase.List.Slice() {
+			if ct := consttype(n); ct < 0 || ct == CTBOOL {
+				continue
+			}
+			tv := typeVal{
+				typ: n.Type.LongString(),
+				val: n.Val().Interface(),
+			}
+			prev, dup := seen[tv]
+			if !dup {
+				seen[tv] = n
+				continue
+			}
+			yyerrorl(ncase.Pos, "duplicate case %s in switch\n\tprevious case at %v",
+				nodeAndVal(n), prev.Line())
 		}
-		setlineno(c.node)
-		yyerror("duplicate case %v in switch\n\tprevious case at %v", prev.Left, prev.Line())
 	}
 }
 
+func nodeAndVal(n *Node) string {
+	show := n.String()
+	val := n.Val().Interface()
+	if s := fmt.Sprintf("%#v", val); show != s {
+		show += " (value " + s + ")"
+	}
+	return show
+}
+
 // walk generates an AST that implements sw,
 // where sw is a type switch.
 // The AST is generally of the form of a linear