cmd/compile: fix detection of duplicate cases for integer ranges

Previously, the check to make sure we only considered constant cases
for duplicates was skipping past integer ranges, because those use
n.List instead of n.Left. Thanks to Emmanuel Odeke for investigating
and helping to identify the root cause.

Fixes #17517.

Change-Id: I46fcda8ed9c346ff3a9647d50b83f1555587b740
Reviewed-on: https://go-review.googlesource.com/31716
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go
index cab0a3d21954f82f6efe7c0c832570a708eb6253..98efedc084c3554649852b47ba5542717bccddae 100644 (file)
--- a/src/cmd/compile/internal/gc/swt.go
+++ b/src/cmd/compile/internal/gc/swt.go
@@ -600,18 +600,21 @@ func (s *exprSwitch) checkDupCases(cc []caseClause) {
        if !s.exprname.Type.IsInterface() {
                seen := make(map[interface{}]*Node)
                for _, c := range cc {
-                       // 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
-                       // (2) it would disallow useful things like
-                       //       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 {
-                               continue
-                       }
-                       if c.node.Left != nil {
+                       switch {
+                       case c.node.Left != nil:
                                // Single constant.
+
+                               // 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
+                               // (2) it would disallow useful things like
+                               //       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 {
+                                       continue
+                               }
+
                                val := c.node.Left.Val().Interface()
                                prev, dup := seen[val]
                                if !dup {
@@ -619,10 +622,9 @@ func (s *exprSwitch) checkDupCases(cc []caseClause) {
                                        continue
                                }
                                setlineno(c.node)
-                               yyerror("duplicate case %v in switch\n\tprevious case at %v", prev.Left, prev.Line())
-                               continue
-                       }
-                       if c.node.List.Len() == 2 {
+                               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.Index(0).Int64()
                                high := c.node.List.Index(1).Int64()
@@ -633,11 +635,12 @@ func (s *exprSwitch) checkDupCases(cc []caseClause) {
                                                continue
                                        }
                                        setlineno(c.node)
-                                       yyerror("duplicate case %v in switch\n\tprevious case at %v", prev.Left, prev.Line())
+                                       yyerror("duplicate case %d in switch\n\tprevious case at %v", i, prev.Line())
                                }
-                               continue
+
+                       default:
+                               Fatalf("bad caseClause node in checkDupCases: %v", c.node)
                        }
-                       Fatalf("bad caseClause node in checkDupCases: %v", c.node)
                }
                return
        }

diff --git a/test/switch5.go b/test/switch5.go
index 54a11b5c9a64c66ebcca01e62d23617e4621e40f..5ca53ba7245e6cdd5422c131504100481ef6730f 100644 (file)
--- a/test/switch5.go
+++ b/test/switch5.go
@@ -90,3 +90,12 @@ func f6() int {
        }
        return 2
 }
+
+// Ensure duplicates in ranges are detected (issue #17517).
+func f7(a int) {
+       switch a {
+       case 0:
+       case 0, 1: // ERROR "duplicate case 0"
+       case 1, 2, 3, 4: // ERROR "duplicate case 1"
+       }
+}