Skip to content

Commit

Permalink
cmd/compile: avoid spurious errors for invalid map key types
Browse files Browse the repository at this point in the history
Instead of trying to validate map key types eagerly in some
cases, delay their validation to the end of type-checking,
when we all type information is present.

Passes go build -toolexec 'toolstash -cmp' -a std .

Fixes #21273.
Fixes #21657.

Change-Id: I532369dc91c6adca1502d6aa456bb06b57e6c7ff
Reviewed-on: https://go-review.googlesource.com/75310
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
  • Loading branch information
griesemer committed Nov 2, 2017
1 parent aec345d commit 25159d3
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 37 deletions.
1 change: 1 addition & 0 deletions src/cmd/compile/internal/gc/align.go
Expand Up @@ -291,6 +291,7 @@ func dowidth(t *types.Type) {

case TFORW: // should have been filled in
if !t.Broke() {
t.SetBroke(true)
yyerror("invalid recursive type %v", t)
}
w = 1 // anything will do
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/compile/internal/gc/main.go
Expand Up @@ -513,6 +513,8 @@ func Main(archInit func(*Arch)) {
fcount++
}
}
// With all types ckecked, it's now safe to verify map keys.
checkMapKeys()
timings.AddEvent(fcount, "funcs")

// Phase 4: Decide how to capture closed variables.
Expand Down
47 changes: 10 additions & 37 deletions src/cmd/compile/internal/gc/typecheck.go
Expand Up @@ -7,7 +7,6 @@ package gc
import (
"cmd/compile/internal/types"
"cmd/internal/objabi"
"cmd/internal/src"
"fmt"
"math"
"strings"
Expand Down Expand Up @@ -420,18 +419,7 @@ func typecheck1(n *Node, top int) *Node {
}
n.Op = OTYPE
n.Type = types.NewMap(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.Pos})
} else if bad.Etype != TANY {
// no need to queue, key is already bad
yyerror("invalid map key type %v", l.Type)
}
}
mapqueue = append(mapqueue, n) // check map keys when all types are settled
n.Left = nil
n.Right = nil

Expand Down Expand Up @@ -3496,16 +3484,18 @@ func stringtoarraylit(n *Node) *Node {
return nn
}

var ntypecheckdeftype int
var mapqueue []*Node

type mapqueueval struct {
n *Node
lno src.XPos
func checkMapKeys() {
for _, n := range mapqueue {
k := n.Type.MapType().Key
if !k.Broke() && !IsComparable(k) {
yyerrorl(n.Pos, "invalid map key type %v", k)
}
}
mapqueue = nil
}

// tracks the line numbers at which forward types are first used as map keys
var mapqueue []mapqueueval

func copytype(n *Node, t *types.Type) {
if t.Etype == TFORW {
// This type isn't computed yet; when it is, update n.
Expand Down Expand Up @@ -3565,7 +3555,6 @@ func copytype(n *Node, t *types.Type) {
}

func typecheckdeftype(n *Node) {
ntypecheckdeftype++
lno := lineno
setlineno(n)
n.Type.Sym = n.Sym
Expand All @@ -3584,22 +3573,6 @@ func typecheckdeftype(n *Node) {
}

lineno = lno

// if there are no type definitions going on, it's safe to
// try to validate the map key types for the interfaces
// we just read.
if ntypecheckdeftype == 1 {
for _, e := range mapqueue {
lineno = e.lno
if !IsComparable(e.n.Type) {
yyerror("invalid map key type %v", e.n.Type)
}
}
mapqueue = nil
lineno = lno
}

ntypecheckdeftype--
}

func typecheckdef(n *Node) {
Expand Down
28 changes: 28 additions & 0 deletions test/fixedbugs/issue21273.go
@@ -0,0 +1,28 @@
// errorcheck

// Copyright 2017 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 main

type T0 T0 // ERROR "invalid recursive type"
type _ map[T0]int

type T1 struct{ T1 } // ERROR "invalid recursive type"
type _ map[T1]int

func f() {
type T2 T2 // ERROR "invalid recursive type"
type _ map[T2]int
}

func g() {
type T3 struct{ T3 } // ERROR "invalid recursive type"
type _ map[T3]int
}

func h() {
type T4 struct{ m map[T4]int } // ERROR "invalid map key"
type _ map[T4]int // ERROR "invalid map key"
}

0 comments on commit 25159d3

Please sign in to comment.