From 352c86eca2991641ded87f9a84520bf90e7a94dc Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 15 Jan 2020 12:30:28 -0800 Subject: [PATCH] go/types: better error messages for range clauses over generic values Change-Id: I7b4ad2734aea5ab508b35ab526f0953b4f99c3f3 --- src/go/types/stmt.go | 82 ++++++++++++++++------------ src/go/types/testdata/stmt0.src | 2 +- src/go/types/testdata/tmp.go2 | 34 +----------- src/go/types/testdata/typeparams.go2 | 12 ++-- 4 files changed, 60 insertions(+), 70 deletions(-) diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go index 223dc5449c..ad86bd5a82 100644 --- a/src/go/types/stmt.go +++ b/src/go/types/stmt.go @@ -752,21 +752,18 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) { var key, val Type if x.mode != invalid { typ := x.typ.Underlying() - // TODO(gri) these tests need to be done also for type parameter channel types - // => move into rangeKeyVal and return an error message instead - if typ, _ := typ.(*Chan); typ != nil { - if typ.dir == SendOnly { - check.softErrorf(x.pos(), "cannot range over send-only channel %s", &x) - // ok to continue - } - if s.Value != nil { - check.softErrorf(s.Value.Pos(), "iteration over %s permits only one iteration variable", &x) - // ok to continue - } + if _, ok := typ.(*Chan); ok && s.Value != nil { + // TODO(gri) this also needs to happen for channels in generic variables + check.softErrorf(s.Value.Pos(), "range over %s permits only one iteration variable", &x) + // ok to continue } - key, val = rangeKeyVal(typ, isVarName(s.Key), isVarName(s.Value)) - if key == nil { - check.softErrorf(x.pos(), "cannot range over %s", &x) + var msg string + key, val, msg = rangeKeyVal(typ, isVarName(s.Key), isVarName(s.Value)) + if key == nil || msg != "" { + if msg != "" { + msg = ": " + msg + } + check.softErrorf(x.pos(), "cannot range over %s%s", &x, msg) // ok to continue } } @@ -861,44 +858,61 @@ func isVarName(x ast.Expr) bool { } // rangeKeyVal returns the key and value type produced by a range clause -// over an expression of type typ; if the range clause is not permitted -// the result is (nil, nil). The wantKey, wantVal flags indicate which of -// the values are actually used - this matters if we range over a generic +// over an expression of type typ, and possibly an error message. If the +// range clause is not permitted the returned key is nil or msg is not +// empty (in that case we still may have a non-nil key type which can be +// used to reduce the chance for follow-on errors). +// The wantKey, wantVal, and hasVal flags indicate which of the iteration +// variables are used or present; this matters if we range over a generic // type where not all keys or values are of the same type. -func rangeKeyVal(typ Type, wantKey, wantVal bool) (Type, Type) { +func rangeKeyVal(typ Type, wantKey, wantVal bool) (Type, Type, string) { switch typ := typ.(type) { case *Basic: if isString(typ) { - return Typ[Int], universeRune // use 'rune' name + return Typ[Int], universeRune, "" // use 'rune' name } case *Array: - return Typ[Int], typ.elem + return Typ[Int], typ.elem, "" case *Slice: - return Typ[Int], typ.elem + return Typ[Int], typ.elem, "" case *Pointer: if typ, _ := typ.base.Underlying().(*Array); typ != nil { - return Typ[Int], typ.elem + return Typ[Int], typ.elem, "" } case *Map: - return typ.key, typ.elem + return typ.key, typ.elem, "" case *Chan: - // TODO(gri) we need to move the additional channel checks here - return typ.elem, Typ[Invalid] + var msg string + if typ.dir == SendOnly { + msg = "send-only channel" + } + return typ.elem, Typ[Invalid], msg case *TypeParam: first := true var key, val Type - if typ.Interface().is(func(t Type) bool { - k, v := rangeKeyVal(t, true, true) + var msg string + typ.Interface().is(func(t Type) bool { + k, v, m := rangeKeyVal(t, wantKey, wantVal) + if k == nil || m != "" { + key, val, msg = k, v, m + return false + } if first { - key, val = k, v + key, val, msg = k, v, m first = false return true } - // TODO(gri) if we fail we should return an explanatory error message - return (!wantKey || Identical(key, k)) && (!wantVal || Identical(val, v)) - }) { - return key, val - } + if wantKey && !Identical(key, k) { + key, val, msg = nil, nil, "all possible values must have the same key type" + return false + } + if wantVal && !Identical(val, v) { + key, val, msg = nil, nil, "all possible values must have the same element type" + return false + } + return true + }) + return key, val, msg } - return nil, nil + return nil, nil, "" } diff --git a/src/go/types/testdata/stmt0.src b/src/go/types/testdata/stmt0.src index ed936aab91..c89ff7f6e4 100644 --- a/src/go/types/testdata/stmt0.src +++ b/src/go/types/testdata/stmt0.src @@ -886,7 +886,7 @@ func rangeloops1() { ee = e _ = ee } - for _ = range sc /* ERROR "cannot range over send-only channel" */ {} + for _ = range sc /* ERROR "send-only channel" */ {} for _ = range rc {} // constant strings diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index e6e99c4986..912029fcb1 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -27,34 +27,6 @@ type II interface{ var _ I = II(nil) */ -func _(type T interface{})(x T) { - for range x /* ERROR cannot range */ {} -} - -func _(type T interface{ type string, []string })(x T) { - for range x {} - for i := range x { _ = i } - for i, _ := range x { _ = i } - for i, e := range x /* ERROR cannot range */ { _ = i } // different element types - for _, e := range x /* ERROR cannot range */ {} // different element types - var e rune - _ = e - for _, (e) = range x /* ERROR cannot range */ {} // different element types -} - - -func _(type T interface{ type string, []rune, map[int]rune })(x T) { - for _, e := range x { _ = e } - for i, e := range x { _ = i; _ = e } -} - -func _(type T interface{ type string, []rune, map[string]rune })(x T) { - for _, e := range x { _ = e } - for i, e := range x /* ERROR cannot range */ { _ = e } // different key types -} - -func _(type T interface{ type string, chan int })(x T) { - for range x {} - for i := range x { _ = i } - for i, _ := range x { _ = i } -} +func _(type T interface{ type string, chan<-int })(x T) { + for i, _ := range x /* ERROR send-only channel */ { _ = i } +} \ No newline at end of file diff --git a/src/go/types/testdata/typeparams.go2 b/src/go/types/testdata/typeparams.go2 index b58e43b9e1..970db54674 100644 --- a/src/go/types/testdata/typeparams.go2 +++ b/src/go/types/testdata/typeparams.go2 @@ -127,11 +127,11 @@ func _(type T interface{ type string, []string })(x T) { for range x {} for i := range x { _ = i } for i, _ := range x { _ = i } - for i, e := range x /* ERROR cannot range */ { _ = i } // different element types - for _, e := range x /* ERROR cannot range */ {} // different element types + for i, e := range x /* ERROR must have the same element type */ { _ = i } + for _, e := range x /* ERROR must have the same element type */ {} var e rune _ = e - for _, (e) = range x /* ERROR cannot range */ {} // different element types + for _, (e) = range x /* ERROR must have the same element type */ {} } @@ -142,7 +142,7 @@ func _(type T interface{ type string, []rune, map[int]rune })(x T) { func _(type T interface{ type string, []rune, map[string]rune })(x T) { for _, e := range x { _ = e } - for i, e := range x /* ERROR cannot range */ { _ = e } // different key types + for i, e := range x /* ERROR must have the same key type */ { _ = e } } func _(type T interface{ type string, chan int })(x T) { @@ -151,6 +151,10 @@ func _(type T interface{ type string, chan int })(x T) { for i, _ := range x { _ = i } // TODO(gri) should get an error here: channels only return one value } +func _(type T interface{ type string, chan<-int })(x T) { + for i := range x /* ERROR send-only channel */ { _ = i } +} + // type inference checks var _ = new() /* ERROR cannot infer T */