cmd/compile: make enqueued map keys fail validation on forward types

Map keys are currently validated in multiple locations but share
a common validation routine. The problem is that early validations
should be lenient enough to allow for forward types while the final
validations should not. The final validations should fail on forward
types since they've already settled.

This change also separates the key type checking from the creation
of the map via typMap. Instead of the mapqueue being populated in
copytype() by checking the map line number, it's populated in the
same block that validates the key type. This isolates key validation
logic while type checking.

Fixes #14988

Change-Id: Ia47cf6213585d6c63b3a35249104c0439feae658
Reviewed-on: https://go-review.googlesource.com/21830Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

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

@@ -372,30 +372,6 @@ func saveorignode(n *Node) {
 	n.Orig = norig
 }
 
-// checkMapKeyType checks that Type key is valid for use as a map key.
-func checkMapKeyType(key *Type) {
-	alg, bad := algtype1(key)
-	if alg != ANOEQ {
-		return
-	}
-	switch bad.Etype {
-	default:
-		Yyerror("invalid map key type %v", key)
-	case TANY:
-		// Will be resolved later.
-	case TFORW:
-		// map[key] used during definition of key.
-		// postpone check until key is fully defined.
-		// if there are multiple uses of map[key]
-		// before key is fully defined, the error
-		// will only be printed for the first one.
-		// good enough.
-		if maplineno[key] == 0 {
-			maplineno[key] = lineno
-		}
-	}
-}
-
 // methcmp sorts by symbol, then by package path for unexported symbols.
 type methcmp []*Field
 

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

@@ -429,10 +429,6 @@ func typChan(elem *Type, dir ChanDir) *Type {
 
 // typMap returns a new map Type with key type k and element (aka value) type v.
 func typMap(k, v *Type) *Type {
-	if k != nil {
-		checkMapKeyType(k)
-	}
-
 	t := typ(TMAP)
 	mt := t.MapType()
 	mt.Key = k

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

@@ -416,6 +416,18 @@ OpSwitch:
 		}
 		n.Op = OTYPE
 		n.Type = typMap(l.Type, r.Type)
+
+		// map key validation
+		alg, bad := algtype1(l.Type)
+		if alg == ANOEQ {
+			if bad.Etype == TFORW {
+				// queue check for map until all the types are done settling.
+				mapqueue = append(mapqueue, mapqueueval{l, n.Lineno})
+			} else if bad.Etype != TANY {
+				// no need to queue, key is already bad
+				Yyerror("invalid map key type %v", l.Type)
+			}
+		}
 		n.Left = nil
 		n.Right = nil
 
@@ -3507,11 +3519,13 @@ func domethod(n *Node) {
 	checkwidth(n.Type)
 }
 
-var (
-	mapqueue []*Node
-	// maplineno tracks the line numbers at which types are first used as map keys
-	maplineno = map[*Type]int32{}
-)
+type mapqueueval struct {
+	n   *Node
+	lno int32
+}
+
+// tracks the line numbers at which forward types are first used as map keys
+var mapqueue []mapqueueval
 
 func copytype(n *Node, t *Type) {
 	if t.Etype == TFORW {
@@ -3520,7 +3534,6 @@ func copytype(n *Node, t *Type) {
 		return
 	}
 
-	mapline := maplineno[n.Type]
 	embedlineno := n.Type.ForwardType().Embedlineno
 	l := n.Type.ForwardType().Copyto
 
@@ -3555,12 +3568,6 @@ func copytype(n *Node, t *Type) {
 	}
 
 	lineno = lno
-
-	// Queue check for map until all the types are done settling.
-	if mapline != 0 {
-		maplineno[t] = mapline
-		mapqueue = append(mapqueue, n)
-	}
 }
 
 func typecheckdeftype(n *Node) {
@@ -3605,12 +3612,13 @@ ret:
 				domethod(n)
 			}
 		}
-
-		for _, n := range mapqueue {
-			lineno = maplineno[n.Type]
-			checkMapKeyType(n.Type)
+		for _, e := range mapqueue {
+			lineno = e.lno
+			if !e.n.Type.IsComparable() {
+				Yyerror("invalid map key type %v", e.n.Type)
+			}
 		}
-
+		mapqueue = nil
 		lineno = lno
 	}
 

test/fixedbugs/issue14988.go

+// errorcheck
+
+// Copyright 2016 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.
+
+// Issue 14988: defining a map with an invalid forward declaration array
+//              key doesn't cause a fatal.
+
+package main
+
+type m map[k]int // ERROR "invalid map key type"
+type k [1]m