From 179e0b3699b2ee0615122609962e8e0787f6ea88 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Fri, 10 Jan 2014 15:21:51 -0800 Subject: [PATCH] go.tools/go/types: only report constants with constant types The issue is addressed with the change in conversions.go. Also: - added corresponding API test - made names for untyped ints, bools consistent with others - use *Basic with names byte/rune instead of uint8/int32 for better output - minor cleanups Fixes golang/go#6949. R=adonovan CC=golang-codereviews https://golang.org/cl/50520043 --- go/types/api_test.go | 89 +++++++++++++++++++++++++++++++++++- go/types/builtins.go | 6 +-- go/types/builtins_test.go | 4 +- go/types/check.go | 1 + go/types/conversions.go | 16 ++++--- go/types/eval_test.go | 4 +- go/types/expr.go | 20 +++----- go/types/predicates.go | 16 +++---- go/types/stmt.go | 2 +- go/types/testdata/errors.src | 6 +-- go/types/typexpr.go | 20 ++++---- go/types/universe.go | 8 +++- 12 files changed, 138 insertions(+), 54 deletions(-) diff --git a/go/types/api_test.go b/go/types/api_test.go index d8747188baf..0547ebce8aa 100644 --- a/go/types/api_test.go +++ b/go/types/api_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + "code.google.com/p/go.tools/go/exact" _ "code.google.com/p/go.tools/go/gcimporter" . "code.google.com/p/go.tools/go/types" ) @@ -40,11 +41,95 @@ func mustTypecheck(t *testing.T, path, source string, info *Info) string { return pkg.Name() } +func TestValues(t *testing.T) { + var tests = []struct { + src string + expr string // constant expression + typ string // constant type + val string // constant value + }{ + {`package a0; const _ = false`, `false`, `untyped bool`, `false`}, + {`package a1; const _ = 0`, `0`, `untyped int`, `0`}, + {`package a2; const _ = 'A'`, `'A'`, `untyped rune`, `65`}, + {`package a3; const _ = 0.`, `0.`, `untyped float`, `0`}, + {`package a4; const _ = 0i`, `0i`, `untyped complex`, `0`}, + {`package a5; const _ = "foo"`, `"foo"`, `untyped string`, `"foo"`}, + + {`package b0; var _ = false`, `false`, `bool`, `false`}, + {`package b1; var _ = 0`, `0`, `int`, `0`}, + {`package b2; var _ = 'A'`, `'A'`, `rune`, `65`}, + {`package b3; var _ = 0.`, `0.`, `float64`, `0`}, + {`package b4; var _ = 0i`, `0i`, `complex128`, `0`}, + {`package b5; var _ = "foo"`, `"foo"`, `string`, `"foo"`}, + + {`package c0a; var _ = bool(false)`, `false`, `bool`, `false`}, + {`package c0b; var _ = bool(false)`, `bool(false)`, `bool`, `false`}, + {`package c0c; type T bool; var _ = T(false)`, `T(false)`, `c0c.T`, `false`}, + + {`package c1a; var _ = int(0)`, `0`, `int`, `0`}, + {`package c1b; var _ = int(0)`, `int(0)`, `int`, `0`}, + {`package c1c; type T int; var _ = T(0)`, `T(0)`, `c1c.T`, `0`}, + + {`package c2a; var _ = rune('A')`, `'A'`, `rune`, `65`}, + {`package c2b; var _ = rune('A')`, `rune('A')`, `rune`, `65`}, + {`package c2c; type T rune; var _ = T('A')`, `T('A')`, `c2c.T`, `65`}, + + {`package c3a; var _ = float32(0.)`, `0.`, `float32`, `0`}, + {`package c3b; var _ = float32(0.)`, `float32(0.)`, `float32`, `0`}, + {`package c3c; type T float32; var _ = T(0.)`, `T(0.)`, `c3c.T`, `0`}, + + {`package c4a; var _ = complex64(0i)`, `0i`, `complex64`, `0`}, + {`package c4b; var _ = complex64(0i)`, `complex64(0i)`, `complex64`, `0`}, + {`package c4c; type T complex64; var _ = T(0i)`, `T(0i)`, `c4c.T`, `0`}, + + {`package c5a; var _ = string("foo")`, `"foo"`, `string`, `"foo"`}, + {`package c5b; var _ = string("foo")`, `string("foo")`, `string`, `"foo"`}, + {`package c5c; type T string; var _ = T("foo")`, `T("foo")`, `c5c.T`, `"foo"`}, + + {`package d0; var _ = []byte("foo")`, `"foo"`, `string`, `"foo"`}, + {`package d1; var _ = []byte(string("foo"))`, `"foo"`, `string`, `"foo"`}, + {`package d2; var _ = []byte(string("foo"))`, `string("foo")`, `string`, `"foo"`}, + {`package d3; type T []byte; var _ = T("foo")`, `"foo"`, `string`, `"foo"`}, + } + + for _, test := range tests { + info := Info{ + Types: make(map[ast.Expr]Type), + Values: make(map[ast.Expr]exact.Value), + } + name := mustTypecheck(t, "Values", test.src, &info) + + // look for constant expression + var expr ast.Expr + for e := range info.Values { + if ExprString(e) == test.expr { + expr = e + break + } + } + if expr == nil { + t.Errorf("package %s: no expression found for %s", name, test.expr) + continue + } + + // check that type is correct + if got := info.Types[expr].String(); got != test.typ { + t.Errorf("package %s: got type %s; want %s", name, got, test.typ) + continue + } + + // check that value is correct + if got := info.Values[expr].String(); got != test.val { + t.Errorf("package %s: got value %s; want %s", name, got, test.val) + } + } +} + func TestCommaOkTypes(t *testing.T) { var tests = []struct { src string - expr string // comma-ok expression string - typ string // typestring of comma-ok value + expr string // comma-ok expression + typ string // comma-ok value type }{ {`package p0; var x interface{}; var _, _ = x.(int)`, `x.(int)`, diff --git a/go/types/builtins.go b/go/types/builtins.go index 35b6078ce29..d362ce5e400 100644 --- a/go/types/builtins.go +++ b/go/types/builtins.go @@ -81,14 +81,14 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b // spec: "As a special case, append also accepts a first argument assignable // to type []byte with a second argument of string type followed by ... . // This form appends the bytes of the string. - if nargs == 2 && call.Ellipsis.IsValid() && x.isAssignableTo(check.conf, NewSlice(Typ[Byte])) { + if nargs == 2 && call.Ellipsis.IsValid() && x.isAssignableTo(check.conf, NewSlice(universeByte)) { arg(x, 1) if x.mode == invalid { return } if isString(x.typ) { if check.Types != nil { - sig := makeSig(S, S, NewSlice(Typ[Byte])) + sig := makeSig(S, S, NewSlice(universeByte)) sig.isVariadic = true check.recordBuiltinType(call.Fun, sig) } @@ -274,7 +274,7 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b switch t := y.typ.Underlying().(type) { case *Basic: if isString(y.typ) { - src = Typ[Byte] + src = universeByte } case *Slice: src = t.elem diff --git a/go/types/builtins_test.go b/go/types/builtins_test.go index 63295226035..fecc531fbe9 100644 --- a/go/types/builtins_test.go +++ b/go/types/builtins_test.go @@ -24,8 +24,8 @@ var builtinCalls = []struct { // Note that ...uint8 (instead of ..byte) appears below because that is the type // that corresponds to Typ[byte] (an alias) - in the other cases, the type name // is chosen by the source. Either way, byte and uint8 denote identical types. - {"append", `var s []byte; _ = append(s, "foo"...)`, `func([]byte, ...uint8) []byte`}, - {"append", `type T []byte; var s T; _ = append(s, "foo"...)`, `func(p.T, ...uint8) p.T`}, + {"append", `var s []byte; _ = append(s, "foo"...)`, `func([]byte, ...byte) []byte`}, + {"append", `type T []byte; var s T; _ = append(s, "foo"...)`, `func(p.T, ...byte) p.T`}, {"cap", `var s [10]int; _ = cap(s)`, `invalid type`}, // constant {"cap", `var s [10]int; _ = cap(&s)`, `invalid type`}, // constant diff --git a/go/types/check.go b/go/types/check.go index b04b701cf39..734cbf6097d 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -96,6 +96,7 @@ func (check *checker) recordTypeAndValue(x ast.Expr, typ Type, val exact.Value) m[x] = typ } if val != nil { + assert(isConstType(typ)) if m := check.Values; m != nil { m[x] = val } diff --git a/go/types/conversions.go b/go/types/conversions.go index 3a057d6f6f0..e90bbbdaef6 100644 --- a/go/types/conversions.go +++ b/go/types/conversions.go @@ -11,9 +11,11 @@ import "code.google.com/p/go.tools/go/exact" // Conversion type-checks the conversion T(x). // The result is in x. func (check *checker) conversion(x *operand, T Type) { + constArg := x.mode == constant + var ok bool switch { - case x.mode == constant && isConstType(T): + case constArg && isConstType(T): // constant conversion switch t := T.Underlying().(*Basic); { case isRepresentableConst(x.val, check.conf, t.kind, &x.val): @@ -45,12 +47,14 @@ func (check *checker) conversion(x *operand, T Type) { // conversion provides the type, per the spec: "A constant may be // given a type explicitly by a constant declaration or conversion,...". final := x.typ - if isUntyped(final) { + if isUntyped(x.typ) { final = T - // For conversions to interfaces, use the argument type's - // default type instead. Keep untyped nil for untyped nil - // arguments. - if isInterface(T) { + // - For conversions to interfaces, use the argument's default type. + // - For conversions of untyped constants to non-constant types, also + // use the default type (e.g., []byte("foo") should report string + // not []byte as type for the constant "foo"). + // - Keep untyped nil for untyped nil arguments. + if isInterface(T) || constArg && !isConstType(T) { final = defaultType(x.typ) } } diff --git a/go/types/eval_test.go b/go/types/eval_test.go index c418e83ab84..7308d50915d 100644 --- a/go/types/eval_test.go +++ b/go/types/eval_test.go @@ -122,7 +122,7 @@ func f(a int, s string) float64 { funcScope := fileScope.Child(0) var tests = []string{ - `true => true, untyped boolean`, + `true => true, untyped bool`, `fmt.Println => , func(a ...interface{}) (n int, err error)`, `c => 3, untyped float`, `T => , p.T`, @@ -132,7 +132,7 @@ func f(a int, s string) float64 { `x => , int`, `d/c => 1, int`, `c/2 => 3/2, untyped float`, - `m.Pi < m.E => false, untyped boolean`, + `m.Pi < m.E => false, untyped bool`, } for _, test := range tests { str, typ := split(test, ", ") diff --git a/go/types/expr.go b/go/types/expr.go index 920b1f1376b..6d62370cb4e 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -447,7 +447,7 @@ func (check *checker) updateExprType(x ast.Expr, typ Type, final bool) { } // Otherwise we have the final (typed or untyped type). - // Remove it from the map. + // Remove it from the map of yet untyped expressions. delete(check.untyped, x) // If x is the lhs of a shift, its final type must be integer. @@ -895,18 +895,20 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) exprKind { if trace { check.trace(e.Pos(), "%s", e) check.indent++ + defer func() { + check.indent-- + check.trace(e.Pos(), "=> %s", x) + }() } kind := check.exprInternal(x, e, hint) // convert x into a user-friendly set of values - record := true var typ Type var val exact.Value switch x.mode { case invalid: typ = Typ[Invalid] - record = false // nothing to do case novalue: typ = (*Tuple)(nil) case constant: @@ -921,18 +923,10 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) exprKind { // delay notification until it becomes typed // or until the end of type checking check.untyped[x.expr] = exprInfo{false, typ.(*Basic), val} - } else if record { - // TODO(gri) ensure that literals always report - // their dynamic (never interface) type. - // This is not the case yet. + } else { check.recordTypeAndValue(e, typ, val) } - if trace { - check.indent-- - check.trace(e.Pos(), "=> %s", x) - } - return kind } @@ -1148,7 +1142,7 @@ func (check *checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind { // (not a constant) even if the string and the // index are constant x.mode = value - x.typ = Typ[Byte] + x.typ = universeByte // use 'byte' name } case *Array: diff --git a/go/types/predicates.go b/go/types/predicates.go index accfe1a7712..025384c0e13 100644 --- a/go/types/predicates.go +++ b/go/types/predicates.go @@ -288,22 +288,20 @@ func isIdenticalInternal(x, y Type, p *ifacePair) bool { // func defaultType(typ Type) Type { if t, ok := typ.(*Basic); ok { - k := t.kind - switch k { + switch t.kind { case UntypedBool: - k = Bool + return Typ[Bool] case UntypedInt: - k = Int + return Typ[Int] case UntypedRune: - k = Rune + return universeRune // use 'rune' name case UntypedFloat: - k = Float64 + return Typ[Float64] case UntypedComplex: - k = Complex128 + return Typ[Complex128] case UntypedString: - k = String + return Typ[String] } - typ = Typ[k] } return typ } diff --git a/go/types/stmt.go b/go/types/stmt.go index 8c3eee64ca1..ac09bf3e01f 100644 --- a/go/types/stmt.go +++ b/go/types/stmt.go @@ -609,7 +609,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { case *Basic: if isString(typ) { key = Typ[Int] - val = Typ[Rune] + val = universeRune // use 'rune' name } case *Array: key = Typ[Int] diff --git a/go/types/testdata/errors.src b/go/types/testdata/errors.src index f8acc0c710b..58b483f0c68 100644 --- a/go/types/testdata/errors.src +++ b/go/types/testdata/errors.src @@ -19,8 +19,8 @@ func f(x int, m map[string]int) { // constants const c1 = 991 const c2 float32 = 0.5 - 0 /* ERROR "0 \(untyped integer constant\) is not used" */ - c1 /* ERROR "c1 \(untyped integer constant 991\) is not used" */ + 0 /* ERROR "0 \(untyped int constant\) is not used" */ + c1 /* ERROR "c1 \(untyped int constant 991\) is not used" */ c2 /* ERROR "c2 \(constant 1/2 of type float32\) is not used" */ c1 /* ERROR "c1 \+ c2 \(constant 1983/2 of type float32\) is not used" */ + c2 @@ -28,7 +28,7 @@ func f(x int, m map[string]int) { x /* ERROR "x \(variable of type int\) is not used" */ // values - x /* ERROR "x != x \(untyped boolean value\) is not used" */ != x + x /* ERROR "x != x \(untyped bool value\) is not used" */ != x x /* ERROR "x \+ x \(value of type int\) is not used" */ + x // value, ok's diff --git a/go/types/typexpr.go b/go/types/typexpr.go index 7541e92ece8..a861d4bdda8 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -123,23 +123,21 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) // If cycleOk is set, e (or elements of e) may refer to a named type that is not // yet completely set up. // -func (check *checker) typ(e ast.Expr, def *Named, cycleOk bool) Type { +func (check *checker) typ(e ast.Expr, def *Named, cycleOk bool) (T Type) { if trace { check.trace(e.Pos(), "%s", e) check.indent++ + defer func() { + check.indent-- + check.trace(e.Pos(), "=> %s", T) + }() } - t := check.typInternal(e, def, cycleOk) - assert(e != nil && t != nil && isTyped(t)) + T = check.typInternal(e, def, cycleOk) + assert(isTyped(T)) + check.recordTypeAndValue(e, T, nil) - check.recordTypeAndValue(e, t, nil) - - if trace { - check.indent-- - check.trace(e.Pos(), "=> %s", t) - } - - return t + return } // funcType type-checks a function or method type and returns its signature. diff --git a/go/types/universe.go b/go/types/universe.go index 2e57b880ed8..52f1678c4ec 100644 --- a/go/types/universe.go +++ b/go/types/universe.go @@ -17,6 +17,8 @@ var ( Universe *Scope Unsafe *Package universeIota *Const + universeByte *Basic + universeRune *Basic ) var Typ = [...]*Basic{ @@ -41,8 +43,8 @@ var Typ = [...]*Basic{ String: {String, IsString, 0, "string"}, UnsafePointer: {UnsafePointer, 0, 0, "Pointer"}, - UntypedBool: {UntypedBool, IsBoolean | IsUntyped, 0, "untyped boolean"}, - UntypedInt: {UntypedInt, IsInteger | IsUntyped, 0, "untyped integer"}, + UntypedBool: {UntypedBool, IsBoolean | IsUntyped, 0, "untyped bool"}, + UntypedInt: {UntypedInt, IsInteger | IsUntyped, 0, "untyped int"}, UntypedRune: {UntypedRune, IsInteger | IsUntyped, 0, "untyped rune"}, UntypedFloat: {UntypedFloat, IsFloat | IsUntyped, 0, "untyped float"}, UntypedComplex: {UntypedComplex, IsComplex | IsUntyped, 0, "untyped complex"}, @@ -185,6 +187,8 @@ func init() { defPredeclaredFuncs() universeIota = Universe.Lookup("iota").(*Const) + universeByte = Universe.Lookup("byte").(*TypeName).typ.(*Basic) + universeRune = Universe.Lookup("rune").(*TypeName).typ.(*Basic) } // Objects with names containing blanks are internal and not entered into