cmd/compile: unify duplicate const detection logic

Consistent logic for handling both duplicate map keys and case values,
and eliminates ad hoc value hashing code.

Also makes cmd/compile consistent with go/types's handling of
duplicate constants (see #28085), which is at least an improvement
over the status quo even if we settle on something different for the
spec.

As a side effect, this also suppresses cmd/compile's warnings about
duplicate nils in (non-interface expression) switch statements, which
was technically never allowed by the spec anyway.

Updates #28085.
Updates #28378.

Change-Id: I176a251e770c3c5bc11c2bf8d1d862db8f252a17
Reviewed-on: https://go-review.googlesource.com/c/152544
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

src/cmd/compile/internal/gc/const.go

@@ -1422,3 +1422,66 @@ func hascallchan(n *Node) bool {
 
 	return false
 }
+
+// A constSet represents a set of Go constant expressions.
+type constSet struct {
+	m map[constSetKey]*Node
+}
+
+type constSetKey struct {
+	typ *types.Type
+	val interface{}
+}
+
+// add adds constant expressions to s. If a constant expression of
+// equal value and identical type has already been added, then that
+// type expression is returned. Otherwise, add returns nil.
+//
+// add also returns nil if n is not a Go constant expression.
+//
+// n must not be an untyped constant.
+func (s *constSet) add(n *Node) *Node {
+	if n.Op == OCONVIFACE && n.Implicit() {
+		n = n.Left
+	}
+
+	if !n.isGoConst() {
+		return nil
+	}
+	if n.Type.IsUntyped() {
+		Fatalf("%v is untyped", n)
+	}
+
+	// Consts are only duplicates if they have the same value and
+	// identical types.
+	//
+	// In general, we have to use types.Identical to test type
+	// identity, because == gives false negatives for anonymous
+	// types and the byte/uint8 and rune/int32 builtin type
+	// aliases.  However, this is not a problem here, because
+	// constant expressions are always untyped or have a named
+	// type, and we explicitly handle the builtin type aliases
+	// below.
+	//
+	// This approach may need to be revisited though if we fix
+	// #21866 by treating all type aliases like byte/uint8 and
+	// rune/int32.
+
+	typ := n.Type
+	switch typ {
+	case types.Bytetype:
+		typ = types.Types[TUINT8]
+	case types.Runetype:
+		typ = types.Types[TINT32]
+	}
+	k := constSetKey{typ, n.Val().Interface()}
+
+	if s.m == nil {
+		s.m = make(map[constSetKey]*Node)
+	}
+	old, dup := s.m[k]
+	if !dup {
+		s.m[k] = n
+	}
+	return old
+}

src/cmd/compile/internal/gc/swt.go

@@ -627,78 +627,24 @@ func checkDupExprCases(exprname *Node, clauses []*Node) {
 	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 !exprname.Type.IsInterface() {
-		seen := make(map[interface{}]*Node)
-		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
-				// (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(n); ct == 0 || ct == CTBOOL {
-					continue
-				}
 
-				val := n.Val().Interface()
-				prev, dup := seen[val]
-				if !dup {
-					seen[val] = n
-					continue
-				}
-				yyerrorl(ncase.Pos, "duplicate case %s in switch\n\tprevious case at %v",
-					nodeAndVal(n), prev.Line())
-			}
-		}
-		return
-	}
-
-	// s's expression is an interface. This is fairly rare, so
-	// keep this simple. Case expressions are only duplicates if
-	// they have the same value and identical types.
-	//
-	// In general, we have to use eqtype to test type identity,
-	// because == gives false negatives for anonymous types and
-	// the byte/uint8 and rune/int32 builtin type aliases.
-	// However, this is not a problem here, because constant
-	// expressions are always untyped or have a named type, and we
-	// explicitly handle the builtin type aliases below.
-	//
-	// This approach may need to be revisited though if we fix
-	// #21866 by treating all type aliases like byte/uint8 and
-	// rune/int32.
-	type typeVal struct {
-		typ *types.Type
-		val interface{}
-	}
-	seen := make(map[typeVal]*Node)
+	var cs constSet
 	for _, ncase := range clauses {
 		for _, n := range ncase.List.Slice() {
-			if ct := consttype(n); ct == 0 || ct == CTBOOL || ct == CTNIL {
+			// 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 n.Type.IsBoolean() {
 				continue
 			}
-			tv := typeVal{
-				typ: n.Type,
-				val: n.Val().Interface(),
-			}
-			switch tv.typ {
-			case types.Bytetype:
-				tv.typ = types.Types[TUINT8]
-			case types.Runetype:
-				tv.typ = types.Types[TINT32]
-			}
-			prev, dup := seen[tv]
-			if !dup {
-				seen[tv] = n
-				continue
+
+			if prev := cs.add(n); prev != nil {
+				yyerrorl(ncase.Pos, "duplicate case %s in switch\n\tprevious case at %v",
+					nodeAndVal(n), prev.Line())
 			}
-			yyerrorl(ncase.Pos, "duplicate case %s in switch\n\tprevious case at %v",
-				nodeAndVal(n), prev.Line())
 		}
 	}
 }

src/cmd/compile/internal/gc/typecheck.go

@@ -8,7 +8,6 @@ import (
 	"cmd/compile/internal/types"
 	"cmd/internal/objabi"
 	"fmt"
-	"math"
 	"strings"
 )
 
@@ -2913,64 +2912,6 @@ func fielddup(name string, hash map[string]bool) {
 	hash[name] = true
 }
 
-func keydup(n *Node, hash map[uint32][]*Node) {
-	orign := n
-	if n.Op == OCONVIFACE {
-		n = n.Left
-	}
-	evconst(n)
-	if n.Op != OLITERAL {
-		return // we don't check variables
-	}
-
-	const PRIME1 = 3
-
-	var h uint32
-	switch v := n.Val().U.(type) {
-	default: // unknown, bool, nil
-		h = 23
-
-	case *Mpint:
-		h = uint32(v.Int64())
-
-	case *Mpflt:
-		x := math.Float64bits(v.Float64())
-		for i := 0; i < 8; i++ {
-			h = h*PRIME1 + uint32(x&0xFF)
-			x >>= 8
-		}
-
-	case string:
-		for i := 0; i < len(v); i++ {
-			h = h*PRIME1 + uint32(v[i])
-		}
-	}
-
-	var cmp Node
-	for _, a := range hash[h] {
-		cmp.Op = OEQ
-		cmp.Left = n
-		if a.Op == OCONVIFACE && orign.Op == OCONVIFACE {
-			a = a.Left
-		}
-		if !types.Identical(a.Type, n.Type) {
-			continue
-		}
-		cmp.Right = a
-		evconst(&cmp)
-		if cmp.Op != OLITERAL {
-			// Sometimes evconst fails. See issue 12536.
-			continue
-		}
-		if cmp.Val().U.(bool) {
-			yyerror("duplicate key %v in map literal", n)
-			return
-		}
-	}
-
-	hash[h] = append(hash[h], orign)
-}
-
 // iscomptype reports whether type t is a composite literal type
 // or a pointer to one.
 func iscomptype(t *types.Type) bool {
@@ -3131,7 +3072,7 @@ func typecheckcomplit(n *Node) (res *Node) {
 		}
 
 	case TMAP:
-		hash := make(map[uint32][]*Node)
+		var cs constSet
 		for i3, l := range n.List.Slice() {
 			setlineno(l)
 			if l.Op != OKEY {
@@ -3145,8 +3086,8 @@ func typecheckcomplit(n *Node) (res *Node) {
 			r = typecheck(r, ctxExpr)
 			r = defaultlit(r, t.Key())
 			l.Left = assignconv(r, t.Key(), "map key")
-			if l.Left.Op != OCONV {
-				keydup(l.Left, hash)
+			if cs.add(l.Left) != nil {
+				yyerror("duplicate key %v in map literal", l.Left)
 			}
 
 			r = l.Right

test/fixedbugs/issue28085.go

+// errorcheck
+
+// Copyright 2018 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
+
+var _ = map[interface{}]int{
+	0: 0,
+	0: 0, // ERROR "duplicate"
+}
+
+var _ = map[interface{}]int{
+	interface{}(0): 0,
+	interface{}(0): 0, // ok
+}
+
+func _() {
+	switch interface{}(0) {
+	case 0:
+	case 0: // ERROR "duplicate"
+	}
+
+	switch interface{}(0) {
+	case interface{}(0):
+	case interface{}(0): // ok
+	}
+}